Re: [PATCH 1/1] linux: open and openat ignore 'mode' with O_TMPFILE in flags

[Roland McGrath <roland at hack dot frob dot com>]

> Introduce a system-specific macro __OPEN_NEEDS_MODE(oflag): by default,
> it tests for O_CREAT in oflag; on Linux, it tests for either O_CREAT or
> __O_TMPFILE (even if __USE_GNU is not defined).

This doesn't really need to be system-specific.  The values of O_*
constants are system-specific, and even what subset of all known O_*
constants are made available is system-specific, but the meaning of each
given O_FOO constant in the API (including things like whether it implies
using a mode argument for open/openat) is part of the generic glibc API.

So certainly a convenience macro is worthwhile to avoid repeating a
nontrivial expression.  But it doesn't need to be system-specific.
It can just be based on #if.

>  __errordecl (__open_missing_mode,
> -	     "open with O_CREAT in second argument needs 3 arguments");
> +	     "open with e.g. O_CREAT in second argument needs 3 arguments");

Keep these explicit about the flags required.  It's OK to say "O_CREAT or
O_TMPFILE" even if O_TMPFILE might not actually exist.  For extra credit,
you could make the checks more specific and different errors for the
O_CREAT and O_TMPFILE cases.  But there is not much benefit to that.

> +      if (__OPEN_NEEDS_MODE(__oflag) && __va_arg_pack_len () < 1)

Space before paren.  I won't cite all the other places with the same issue.

>  /* Open FILE and return a new file descriptor for it, or -1 on error.
> -   OFLAG determines the type of access used.  If O_CREAT is on OFLAG,
> +   OFLAG determines the type of access used.  If __OPEN_NEEDS_MODE(OFLAG),
>     the third argument is taken as a `mode_t', the mode of the created file.

Don't make comment changes like this.  The comments in the public header
file are there to be user documentation of the API.  So they should never
mention internal symbols or whatnot, only the aspects of the API that
someone writing a call to this function would use directly.  Again, it's
fine to say "... or O_TMPFILE" without further qualification regardless of
whether O_TMPFILE is actually available.


Thanks,
Roland