This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


> +	* io/fcntl.h: Define __OPEN_NEEDS_MODE.

	* io/fcntl.h (__OPEN_NEEDS_MODE): New macro.

> +	* io/bits/fcntl2.h (open): Use __OPEN_NEEDS_MODE.

Appearing right after the first line, this can say just "Use it."  That's
shorter, but there is nothing wrong with being explicit as you were.  
Your choice.

> +#ifdef __O_TMPFILE
> +# define __OPEN_NEEDS_MODE(oflag) \
> +  ( ((oflag) & O_CREAT) != 0 || ((oflag) & __O_TMPFILE) == __O_TMPFILE )
> +#else
> +# define __OPEN_NEEDS_MODE(oflag) ( ((oflag) & O_CREAT) != 0 )
> +#endif

Drop the extra spaces around the parens.  We use a space between an
identifier (or keyword) and an open-paren, but not after an open-paren or
before a close-paren.

>  /* 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,
> -   the third argument is taken as a `mode_t', the mode of the created file.
> +   OFLAG determines the type of access used.  If O_CREAT or O_TMPFILE is on
> +   OFLAG, the third argument is taken as a `mode_t', the mode of the created
> +   file.

The old text had this problem before, but it's always good to clean up
verbiage while you're there: "is on OFLAG" should be "is set in OFLAG".

With those cosmetic nits fixed, this change is fine to go in now on its
substance.  I don't see copyright paperwork from you.  Have you begun the
process with the FSF already or do you not know what I'm talking about?
We'll need the assignment in place before we can accept your patch (which
we'd very much like to accept now!).  
See https://sourceware.org/glibc/wiki/Contribution%20checklist


Thanks,
Roland


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]