Re: [PATCH] fs: Remove duplicated flag from VALID_OPEN_FLAGS

[Krzysztof Wilczynski <kw@linux.com>]

On 20-05-22 16:47:19, Al Viro wrote:

Hello Al,

Thank you for review.  This is going to be an invaluable learning for
experience me.  Also, apologies for causing you to do more work.

[...]
> Now ask yourself what might be the reason for that "duplicated argument".
> Try to figure out what the values of those constants might depend upon.
> For extra points, try to guess what has caused the divergences.
>
> Please, post the result of your investigation in followup to this.

I had a look at O_NONBLOCK and O_NDELAY, and the values of these are
platform-specific, as per:

include/uapi/asm-generic/fcntl.h:
  #define O_NONBLOCK      00004000
  #define O_NDELAY	O_NONBLOCK

arch/sparc/include/uapi/asm/fcntl.h:
  #define O_NONBLOCK  0x4000
  #if defined(__sparc__) && defined(__arch64__)
  #define O_NDELAY    0x0004
  #else
  #define O_NDELAY    (0x0004 | O_NONBLOCK)
  #endif

arch/alpha/include/uapi/asm/fcntl.h:
  #define O_NONBLOCK   00004

arch/mips/include/uapi/asm/fcntl.h:
  #define O_NONBLOCK   0x0080

For Sparc, there we handle it as a special case, for example:

linux/fs/ioctl.c ->
  ioctl_fionbio():

  #ifdef __sparc__
          /* SunOS compatibility item. */
          if (O_NONBLOCK != O_NDELAY)
                  flag |= O_NDELAY;
  #endif

There is also a comment in the fs/fcntl.c that adds clarification:

fs/fcntl.c ->
  fcntl_init():

  /*
   * Please add new bits here to ensure allocation uniqueness.
   * Exceptions: O_NONBLOCK is a two bit define on parisc; O_NDELAY
   * is defined as O_NONBLOCK on some platforms and not on others.
   */

The behaviour of O_NONBLOCK and O_NDELAY also is platform-dependent,
where O_NDELAY might just return 0 instead of returning an error and
setting errno to either EAGAIN or EWOULDBLOCK.

I also took a look at commit 80f18379a7c3 ("fs: add a VALID_OPEN_FLAGS")
that introduced the new definition.

After looking at the warning coming from Coccinelle, I had a look and
assumed that flag O_NDELAY was added to the VALID_OPEN_FLAGS twice
accidentally.

I am still unsure why we would want to keep O_NDELAY twice?  Setting the
same bits multiple would be a non-operation to the best of my knowledge.

But, I sincerely apologise for a not very clear commit message and not
mentioning the flag in the subject and explaining what has been done
better.  I will send a v2, if that is OK with you?

Again, thank you for you time!

Krzysztof