Re: Copy-edit comments on filexfer-08

[der Mouse <mouse%Rodents.Montreal.QC.CA@localhost>]

> (I believe this round introduced one incompatible change.  [...])

It might help to call out what it is.  I assume you're talking about
the REALPATH change, but it could also be the version-select switch
from uint32 to string.  (Your reply makes it clear *you've* been
thinking of the latter as a string, but that's not what it said.)

>     The use of additional packet types in the non-extension range MUST be
>     introduced through IETF consensus.  [...]

Thank you, that helps greatly.  The "in the non-extension range"
semantics were missing from the previous language - or at least,
unclear enough that I read them as missing - and it makes all the
difference.  (The rest of the wording changes are less critical, though
I find the new text significantly better nevertheless.)

>> In 4.5, supported-open-block-masks and supported-block-masks are
>> uint64s.  Is there any reason they aren't simply 16-bit fields
>> (probably uint32 on the wire), with each bit indicating whether a
>> particular combination is supported?
> Well, I set out to do it that way originally, but it was easier to
> specify this way.  Can you come up with some language that specs it
> reasonably clearly with adding a whole bunch of new identifiers?

Probably.  Let's see, how about this (using supported-open-block-masks
for the sake of argument):

           uint32 supported-open-block-masks

(actually, I'd prefer uint16, but I don't think we have one - maybe put
both block-masks in a single uint32?)

   supported-open-block-masks
      A 16-bit mask specifying which combinations of blocking flags are
      supported.  Each bit corresponds to one combination of the
      SSH_FXF_ACCESS_BLOCK_READ, SSH_FXF_ACCESS_BLOCK_WRITE,
      SSH_FXF_ACCESS_BLOCK_DELETE, and SSH_FXF_ACCESS_BLOCK_ADVISORY
      bits from Section 7.1.1.3, with a set bit corresponding to a
      supported combination and a clear bit an unsupported combination.
      The index of a bit, bit zero being the least significant bit,
      viewed as a four-bit number, corresponds to a combination of flag
      bits, shifted right so that BLOCK_READ is the least significant
      bit.  The combination `no blocking flags' MUST be supported, so
      the low bit will always be set.

      For example, a server supporting only the classic advisory read
      (shared) and write (exclusive) locks would set the bits
      corresponding to READ+WRITE+ADVISORY, 0b1011, and WRITE+ADVISORY,
      0b1010, plus the always-set bit 0b0000, giving a mask value of
      0b0000110000000001, or 0x0c01; a server supporting no locking at
      all would set only bit zero, giving 0x0001.

      Bits other than the low 16 are reserved for possible future use
      and MUST NOT be set.

The last sentence does not apply if both masks are packed into a single
uint32, of course.

> I also suspect that it will be easier to use as a series of bitmasks.

Well, as a code author, if I had to deal with the masks as specified,
I'd use the 16-bit mask form internally, converting to the spec's form
very late on output and very early on input.  I suspect this says more
about what kind of internal data structures I prefer (and to some
extent what language I'd likely be using) than about anything else....

> I origanally had wording specifying that it should be padded out with
> 0b0000, but it got lost somewhere.

Well, if the current spec is kept, I think it needs to be found. :)

> I've changed it to read:

>     Typically, the client SHOULD NOT down-grade the protocol version
>     using this extension; however, it is not forbidden to do so.  One
>     reason a client might do so is to work around a buggy implementation.

> Is that better?

Somewhat.  There is still the implication that of two versions, one
will be a downgrade compared to the other, which, even if it's true
now, surely will not remain true forever.  However, I think the
implication is loose enough that that language is OK.

I'm still not clear whether totally non-numeric version strings are
acceptable or not.  I see nothing that would forbid, say,
"illuminati%fnord.mil@localhost" from appearing in the list, nor any simple way
to decide whether it would be a downgrade as compared to
"infibulated-gonkulator%example.net@localhost".  But if you're going to allow DNS
extensibility at all, that is, ultimately, unavoidable - it's entirely
possible that of two "private" versions, neither is a feature subset of
the other, regardless of how they're named.

>> It is not clear whether DELETE_ON_CLOSE is permitted to destroy the
>> name immediately even if it does not destroy the file, [...]
> Hmmm... in this case, I think I'm inclined to make it undefined
> whether the filename is removed immediately or on close.

I think that's probably the route I'd go.

> Thanks again for you reviews.

No problem!

/~\ The ASCII				der Mouse
\ / Ribbon Campaign
 X  Against HTML	       mouse%rodents.montreal.qc.ca@localhost
/ \ Email!	     7D C8 61 52 5D E7 2D 39  4E F1 31 3E E8 B3 27 4B