Re: A proposal for OPEN

[Jeffrey Hutzelman <jhutz%cmu.edu@localhost>]

On Monday, September 08, 2003 17:37:46 -0600 Joseph Galbraith 
<galb%vandyke.com@localhost> wrote:

> byte   SSH_FXP_OPEN
> string filename
> uint32 access_desired
> uint32 access_flags
> attrib open_attrs
>
> access_flags
>     The access_flags is two seperate bit fields.  Bit values
>     in the 0xFFFF0000 mask are required, and if they can not
>     be supported, the operation MUST fail.
>
>     Bit values in the 0x0000FFFF mask are requested, but if they
>     can not be supported the operation SHOULD continue.
>
>     Clients MUST NOT send any undefined bits in the access_flags
>     field.  Servers should ignore any bits not specified here.
>
>     Future IETF action may add additional bits without
>     reving the version number.

This isn't really self-consistent.  Are bits in the 0xffff0000 group 
critical or not?  Given that new bits can be defined without bumping the 
version number, it's important that the behaviour of a server depend on 
whether the server supports a given flag, and not whether the server was 
implemented before or after a given flag was defined.

Given that you've indicated you want to be able to add flags without 
bumping the version, and that you've gone out of your way to indicate which 
potential future flags would be critical and which would not, I'd say you 
do want servers to reject critical flags of which they're unaware.


I'd suggest something like this:

The access_flags is two separate bit fields.  Bit values in the 0xffff0000 
mask are required, while bit values in the 0x000ffff mask are requested. 
Clients must not send any undefined bits in the access_flags field.

Bit values in the 0xffff0000 mask are required; if a flag in this range is 
set and cannot be supported by the server, the operation MUST fail.

Bit values in the 0x0000ffff mask are requested; if a flag in this range is 
set and its meaning is not known to the server, the operation MUST continue 
as if the flag had not been set.  If such a flag is set and its meaning is 
known to the server, but cannot be supported, the server MAY report an 
appropriate error or allow the operation to continue as if the flag had not 
been set.

Future IETF action may add additional bits without any need to change the 
protocol version number.

[ I think something slightly stronger is warranted -- the available space 
is small, and should probably only be used for "mainstream" revisions of 
the protocol and not private extensions. ]



>     The following required fields are defined:
>         ACCESS_DISPOSITION    = 0x00070000
>             CREATE_NEW        = 0x00000000
>             CREATE_OVERWRITE  = 0x00010000
>             OPEN_EXISTING     = 0x00020000
>             OPEN_OR_CREATE    = 0x00030000
>             TRUNCATE_EXISTING = 0x00040000
>         ACCESS_APPEND_DATA    = 0x00080000

The meanings of these are more or less obvious, but they should be spelled 
out, I think.  In terms of traditional UNIX open flags, I think these mean:

CREATE_NEW = O_CREAT | O_EXCL
CREATE_OVERWRITE = O_CREAT | O_TRUNC
OPEN_EXISTING = 0
OPEN_OR_CREATE = O_CREAT
TRUNCATE_EXISTING = O_TRUNC

ACCESS_APPEND_DATA = O_APPEND

Is this correct?

Note that the semantics of ACCESS_APPEND_DATA are not entirely clear, 
especially when used with CREATE_NEW, CREATE_OVERWRITE, or 
TRUNCATE_EXISTNG.  Does this merely indicate that the file position pointer 
is positioned to the EOF when the file is opened, or does it mean that 
every write is performed atomically at the EOF, without changing the file 
position pointer and without any chance of conflict with another appending 
process?

If the former, then the flag has no effect in cases where the file will 
have 0 length after the open, and we shoulld drop it entirely and instead 
defined dispositions APPEND_EXISTING and APPEND_OR_CREATE.

If the latter, this needs to be spelled out, and it should be clear that if 
a server cannot actually support the atomic-append behaviour, it MUST fail 
the operation.  Of course, that leaves you with no means of getting a 
non-atomic append if that is sufficient.


>     The following required fields are defined:
>         ACCESS_SHARE_READ    = 0x00000001
>         ACCESS_SHARE_WRITE   = 0x00000002
>         ACCESS_SHARE_DELETE  = 0x00000004

The meanings of these are not obvious to me, probably because they're drawn 
from some Windows terminology with which I'm not familiar.


>     I've chosen the DISPOSITION value over the previous
>     CREAT/TRUNC/EXCL bit mask because it:
>
>     a. takes the same three bits
>     b. removes the ability to specify poorly defined
>        combinations (TRUNC|EXCL)
>     c. I think it is easier to understand.
>     d. Has room for expanision if we need it

Yes; this sounds quite reasonable.  The interactions of O_CREAT, O_EXCL, 
and O_TRUNC will be familiar to UNIX folk, but not to others, and the 
combination of TRUNC and EXCL is indeed ill-defined.

-- Jeff