Re: Loaded footgun open_datasync on Windows

On Wed, Jun 6, 2018 at 3:06 PM, Kuntal Ghosh <kuntalghosh(dot)2007(at)gmail(dot)com>
wrote:

> On Wed, Jun 6, 2018 at 2:39 PM, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
> wrote:
> > On Wed, Jun 6, 2018 at 10:18 AM, Michael Paquier <michael(at)paquier(dot)xyz>
> > wrote:
> >>
> >> On Wed, Jun 06, 2018 at 09:58:34AM +0530, Amit Kapila wrote:
> >>
> >> >> 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.
> >>
> >> Which is what basically happens on any *nix platform, are you foreseeing
> >> anything bad here?
> >>
> >>
> >
> > Nothing apparent, but I think we should try to find out why at the first
> > place this has been made backend specific.
> >
> It seems the "#ifndef FRONTEND" restriction was added around
> pgwin32_open() for building libpq with Visual C++ or Borland C++. The
> restriction was added in commit 422d4819 to build libpq with VC++[1].
> Later, in commit fd7c3f67e0bc4, the support for Borland C++ was also
> added.
>
> So, I'm not sure whether removing that restriction will work for all
> front-end modules.
>
>
Thanks for doing investigation. I agree with you that it appears from the
old discussion that we have added this restriction to build libpq/psql
(basically frontend) modules. However, I tried to build those modules on
windows by removing that restriction and it doesn't give me any problem
(except one extra argument error, which can be easily resolved). It might
be that it gives problem with certain version of MSVC. I think if we want
to pursue this direction, one needs to verify on various supportted
versions (of Windows) to see if things are okay.

Speaking about the issue at-hand, one way to make pg_test_fsync work in to
just include c.h and remove inclusion of postgres_fe.h, but not sure if
that is the best way. I am not sure if we have any other options to
proceed in this case.

Thoughts?

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