Re: [PATCH] msvc: Fix compilation error due to missing mktemp() declaration

From: Erik Faye-Lund <kusmabite@gmail.com>
Date: Dec 30, 2010, 21:47 UTC
Message-ID: <AANLkTinAAGWeFDomo-NQrVtrdf9ue1xVxufy+bkWz1Fc@mail.gmail.com>
In-Reply-To: <4D1CDE84.7090402@ramsay1.demon.co.uk>

On Thu, Dec 30, 2010 at 8:33 PM, Ramsay Jones <ramsay@ramsay1.demon.co.uk> wrote:

Show 30 quoted lines

> Erik Faye-Lund wrote:
>> On Thu, Dec 23, 2010 at 8:05 PM, Ramsay Jones
>> <ramsay@ramsay1.demon.co.uk> wrote:
>>> Commit d1b6e6e (win32: use our own dirent.h, 2010-11-23) removed
>>> the compat/vcbuild/include/dirent.h compatibility header file.
>>> This file, among other things, included the <io.h> system header
>>> file which provides the declaration of the mktemp() function.
>>>
>>> In order to fix the compilation error, we add an include directive
>>> for <io.h> to the compat/vcbuild/include/unistd.h header. (The
>>> MinGW build includes <io.h> from it's <unistd.h> header too.)
>>>
>>> Signed-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
>>> ---
>>
>> Thanks. But shouldn't this header be included in mingw.h (or perhaps
>> msvc.h) because of _get_osfhandle and _commit?
>
> Er... no. ;-)
>
> All uses of _get_osfhandle() and _commit() on the msvc build (after this
> patch) are within the scope of an appropriate declaration, so there is
> no *need* to include <io.h> in either mingw.h or msvc.h.
> [I'm confident the same is true of the MinGW build as well, but I didn't
> have time to check before sending this mail...]
>
> I suspect that you already know this and I'm just being dumb in missing
> the import of your question... Perhaps you could clarify your concerns
> regarding this patch?
>

Actually, I suspect that you know exactly what my concern is (based on the above), and that it's not with your patch.

I have a patch in my MSVC-tree that includes io.h in mingw.h, because mingw.h already depends on facilities from io.h on MSVC. My point was simply that this dependency was already present, and as such I think mingw.h is the appropriate place to include it.

Your patch might remove the warnings (perhaps it was only msvc.c that depends on io.h, I don't remember), and that's fine. But I think we should solve the problem in msvc.[ch] instread, as this is the compatibility-layer that the rest of the system includes.

But it's just an opinion, deal with it as you please. The most important part is to prevent warnings/errors, how we do it is secondary.