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

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

[Ramsay Jones]

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>
---
 compat/vcbuild/include/unistd.h |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/compat/vcbuild/include/unistd.h b/compat/vcbuild/include/unistd.h
index b14fcf9..a61160f 100644
--- a/compat/vcbuild/include/unistd.h
+++ b/compat/vcbuild/include/unistd.h
@@ -1,6 +1,8 @@
 #ifndef _UNISTD_
 #define _UNISTD_
 
+#include <io.h>
+
 /* Win32 define for porting git*/
 
 #ifndef _MODE_T_
-- 
1.7.3

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

[Erik Faye-Lund]

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?

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

[Ramsay Jones]

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?

ATB,
Ramsay Jones

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

[Erik Faye-Lund]

On Thu, Dec 30, 2010 at 8:33 PM, Ramsay Jones
<ramsay@ramsay1.demon.co.uk> wrote:
> 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.

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

[Ramsay Jones]

Erik Faye-Lund wrote:
> Actually, I suspect that you know exactly what my concern is (based on
> the above), and that it's not with your patch.

Actually no, which is why I asked! :-D

> 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.

OK, v2 of patch on the way...

ATB,
Ramsay Jones