Re: [git pull] vfs.git part 2

[Rasmus Villemoes <linux@rasmusvillemoes.dk>]

Linus Torvalds <torvalds@linux-foundation.org> writes:

> On Fri, Jul 12, 2013 at 12:39 PM, Al Viro <viro@zeniv.linux.org.uk> wrote:
>>
>> [suggested by Rasmus Villemoes] make O_DIRECTORY | O_RDWR part of O_TMPFILE;
>> that will fail on old kernels in a lot more cases than what I came up with.
>
> So see earlier about why I'm not convinced about O_RDWR. But even if
> we really want that (and it might be better to start off too narrow
> than accept anything else) your patch tests those bits the wrong way
> (any O_RDWR test should be done using the O_ACCMODE mask, not using
> the O_RDWR value itself as a mask)

On further thought, I think I'll retract the suggestion to include
O_RDWR in O_TMPFILE: If that is done, I don't see how one can ever allow
O_WRONLY functionality without breaking the ABI (or introducing
O_TMPFILE_WRONLY). So I suggest O_TMPFILE == O_DIRECTORY|__O_TMPFILE,
and correct use is open(path, O_TMPFILE|O_RDWR, mode).

Also, O_TMPFILE without O_RDWR (or O_WRONLY) should then give -EINVAL. 

Pros:

The access mode is explicit
Easy to allow O_TMPFILE|O_WRONLY in the future (or now?)
Static code checkers complaining about the lack of
O_{RDONLY,RDWR,WRONLY} in open() (?)

Cons:

We catch one fewer case on older kernels, but not one which is likely to
occur in real programs: On old kernels, O_TMPFILE|O_RDONLY, or
equivalently O_TMPFILE, may give a valid file descriptor (in the case
where path does name a directory). However: Anyone writing a program
using O_TMPFILE probably at least tests on a kernel knowing about that,
and so would be given the -EINVAL error, and the documentation can just
spell out that O_TMPFILE is not compatible with O_RDONLY.

Rasmus