On Tue, Jun 05, 2018 at 04:15:00PM +0200, Laurenz Albe wrote: > The attached patch makes pg_test_fsync use pgwin32_open on Windows, which is > what > we use elsewhere.
Well, pg_upgrade may benefit from that as one example, as any other tools. > That should fix the problem. > Ist there a better way to do this? The problem is that "c.h" is only included > at the very end of "postgres-fe.h", which makes front-end code use "open" > rather than "pgwin32_open" on Windows. And port.h is included at the end of c.h. > Having read it again, I think that the documentation is fine as it is: > After all, this is just advice what you can do if you are running on unsafe > hardware, > which doesn't flush to disk like it should. People tend to ignore this part from the docs. Well I won't fight hard on that if folks don't want to change that... > --- 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? 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 which position is better here, but thinking that all in-core frontend code could benefit from a couple of simplifications that could be worth the shot. -- Michael
signature.asc
Description: PGP signature
