Re: Loaded footgun open_datasync on Windows

On Wed, Jun 6, 2018 at 8:28 AM, Michael Paquier <michael(at)paquier(dot)xyz> wrote:

> On Tue, Jun 05, 2018 at 04:15:00PM +0200, Laurenz Albe wrote:
>
> > --- a/src/bin/pg_test_fsync/pg_test_fsync.c
> > +++ b/src/bin/pg_test_fsync/pg_test_fsync.c
> > @@ -3,6 +3,8 @@
> > * tests all supported fsync() methods
> > */
> >
> > +/* we have to include c.h first so that pgwin32_open is used on Windows
> */
> > +#include "c.h"
> > #include "postgres_fe.h"
> >
> > #include <sys/stat.h>
>
> Ouch. Including directly c.h as you do here is against project policy
> code. See recent commit a72f0365 for example. pgwin32_open() is
> visibly able to handle frontend code if I read this code correctly, so
> could we just remove the "#ifndef FRONTEND" restriction?

I think we need to explore a bit, if we want to remove that, for example
some of the frontend modules (like pg_receivewal, pg_recvlogical) call two
argument open which would require change.

> It could be
> risky for existing callers of open() for tool maintainers, or on the
> contrary people could welcome a wrapper of open() which is
> concurrent-safe in their own tools.

I am not sure if we can safely assume that because using these functions
would allow users to concurrently delete the files, but may be it is okay
for all the FRONTEND modules. One another alternative could be that we
define open as pgwin32_open (for WIN32) wherever we need it.

--
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com