Re: Safeguards against incorrect fd flags for fsync()

Hi,

On Wed, Oct 09, 2019 at 03:26:40PM +0900, Michael Paquier wrote:
> After the set of issues discussed here, it seems to me that it would
> be a good thing to have some safeguards against incorrect flags when
> opening a fd which would be used for fsync():
> https://www.postgresql.org/message-id/16039-196fc97cc05e141c@postgresql.org
>
> Attached is a patch aimed at doing that. Historically O_RDONLY is 0,
> so when looking at a directory we just need to make sure that no write
> flags are used. For files, that's the contrary, a write flag has to
> be used.
>
> Thoughts or better ideas?

Well O_RDONLY might historically be 0 almost everywhere, but it is
defined to 1 on the GNU system [1]:

|#define O_RDONLY 0x0001 /* Open read-only. */

So there, the comparison with 0 does not work and initdb (at least)
fails on assert-enabled builds:

|running bootstrap script ... TRAP: FailedAssertion("(desc_flags & (O_RDWR | O_WRONLY)) == 0", File: "fd.c", Line: 395, PID: 4560)

TTBOMK, POSIX does not mandate that O_RDONLY be 0, so I think this check
is overly zealous. The better way might be to mask the flags with
O_ACCMODE and then just check what you want, like in the attached.

Thoughts?

Michael

[1] https://sourceware.org/git/?p=glibc.git;a=blob;f=sysdeps/mach/hurd/bits/fcntl.h;hb=HEAD#l39