On Thu, May 31, 2012 at 2:13 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Robert Haas <robertmhaas@gmail.com> writes:
>> On Thu, May 31, 2012 at 8:15 AM, Johann 'Myrkraverk' Oskarsson
>> <johann@2ndquadrant.com> wrote:
>>> Is this something to discuss with the MinGW W64 team?
>
>> My viewpoint on this (which is different than Tom's) is that we're
>> probably not entitled to assume anything about what the system header
>> files do with respect to stat. On some systems, they might just have
>> a function prototype, while others might define stat or stat() as a
>> macro. It seems to me that our source code is hoping for a function
>> definition rather than a macro definition and falling over when that's
>> not how it is. I don't see that as very reasonable, unless we have
>> some basis for believing that the OS isn't entitled to define stat as
>> a macro rather than a function, and maybe not even then.
>
> I quote from the POSIX:2008 specification for <sys/stat.h>:
>
> The following shall be declared as functions and may also be
> defined as macros. Function prototypes shall be provided.
> ...
> int stat(const char *restrict, struct stat *restrict);
>
> I do not believe that the standard intends the word "shall" to have any
> wiggle room. I would also read this to mean that if the header defines
> "stat" as a macro, that macro ought to be an alternative way of invoking
> the function.
There's nothing in the passage you quote that says the macro
definition can't do anything other than invoke the eponymous function,
but...
> Now we are messing up by failing to #undef the macro
> before redefining it, but if we do that and it still doesn't work, the
> header is not conformant to POSIX.
...this is probably still true.
>> We have
>> plenty of other places where we use our own wrapper function in lieu
>> of OS facilities for various reasons (e.g. BasicOpenFile) and I don't
>> think adding one more is a big deal.
>
> Well, I think it is. Where we use a wrapper function, it's because it
> has somewhat different semantics from the underlying standard function.
> I do not think it's good for readability to define pgfoo() when that's
> only meant to be exactly foo(). I'm especially not keen on doing that
> just because one variant of MinGW has managed to break their conformance
> with POSIX.
In this case, I feel like we've kind of already stepped in it, because
we've defined stat() to be pgwin32_safestat(), which is, in fact, not
exactly stat(). So right now ISTM that a naive backend hacker might
think that stat() means "the stat provided by the OS", but, on
Windows, it doesn't. I would exactly describe myself as "keen" on
adding wrapper functions to things like stat(), but until non-POSIX
operating systems go the way of the dodo bird, I'm not sure there's
any way around it.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company