Chaosnet / chaosnet-tools

Tools for Chaosnet
http://chaosnet.net/
3 stars 1 forks source link

Put error message at right offset #13

Closed bictorv closed 8 months ago

bictorv commented 8 months ago

and give better error messages for mount failures.

larsbrinkhoff commented 7 months ago

According to @Bikeman, the offset change breaks DUMP on ITS.

Bikeman commented 7 months ago

Yeah indeed. have a look at

https://github.com/PDP-10/its/blob/f25c6e9838526133953f3e84cd39da0bd5cecfa1/src/syseng/dump.449#L7895C10-L7895C27

The minimum length of the RTAPE status message is 36 bytes. If unconvinced try a _remote :DUMP to a non-existing tape in read-only mode: In the current version, this will then print an error message in ITS with the first character missing...because the message is expected (if any) at offset 36, not 35

Just guessing here, but is it possible that the message is prefixed by its length at offset 35?

bictorv commented 7 months ago

After reviewing the code in DUMP and Open Genera, I stand corrected. The DRIVE field is 1 length byte followed by 16 bytes of drive name, not 15. (I'll correct the code in the CADR RTAPE implementation.)

But the error message has no length byte (see DUMP, and also Open Genera).

Whoever fixes this in rtape.c, pleeease take the time to define constants for the status offsets?

Bikeman commented 7 months ago

So just to be clear, that means the FLAGS field moves up one byte to offset 34. and 35. ? I guess this must have worked before because the message is initialized to 0 and the flags all zero means "OK".

Sorry I have just started looking at the PDP-10 and I'm not fluent yet in it's assembly language.

As far as improving rtape.c (besides constants for offsets) is concerned: the current implementation of sent_status leaves bytes 3 ... 16 initialized to zero, I think they would normally contain some information similar to what is shown here: https://gunkies.org/wiki/Chaos_RTAPE_protocol

(Then again, as we now know, their format of the status message is also wrong. )

So is it save to leave all this info like "Number of blocks read " set to zero no matter what?

bictorv commented 7 months ago

So just to be clear, that means the FLAGS field moves up one byte to offset 34. and 35. ?

It does.

bytes 3 ... 16 initialized to zero, I think they would normally contain some information

If it's easy to do, sure! But you can also take one thing at a time and start with the offsets. I'd be happy to review your PR!

larsbrinkhoff commented 7 months ago

When I wrote rtape.c I only implemented what's needed for ITS' DUMP, hence some fields are left empty. But sure, they can be filled in.

Bikeman commented 7 months ago

Well if DUMP doesn't use those fields, it's fine for me, good to know.

@bictorv

I'd be happy to review your PR!

Thanks a lot! OK, so as you might have seen I've forked this repo and will do my work there, I'll let you know when I have something to merge. I really think this rtape tool is really useful, also in connection with the itstar tool that is already in the PiDP-10 image (as source at least).

P.S.: Until the cbridge firewall is implemented, I was thinking about adding an optional argument that would force the drivename to start with a user supplied string (or else reject the operation). Kind of a poor man's access password.

larsbrinkhoff commented 7 months ago

This is exactly why I wrote rtape.c in the first place, so I'm glad you find it useful.

You may also be interested in my automated backup program in the LARS directory. You can assemble it to e.g. DRAGON;WEEKLY BACKUP to have it run once a week. I'm not sure it runs 100% reliably, though. Controlling inferior job TTY input and output isn't easy in ITS.

Bikeman commented 6 months ago

I have some more thoughts about the rtape tool but I think the chaosnet-tools repo is the better place to discuss this perhaps? https://github.com/Chaosnet/chaosnet-tools/issues/15

bictorv commented 6 months ago

The cbridge firewall has been implemented, by the way. I'll merge the stuff now.

Bikeman commented 2 months ago

Upps, I noticed I had never come back with a pull request wrt the discussion above. I now created one. Feel free to cherry-pick if you want , the new feature that gives a bit of more freedom to how filenames are created and mapped might be a bit over the top but anyway the default behavior should be unchanged.