Re: [RFC PATCH 2/2] MSVC: Fix an "incompatible pointer types" compiler warning

From: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
Date: Dec 4, 2009, 22:50 UTC
Message-ID: <4B199239.2050709@ramsay1.demon.co.uk>
In-Reply-To: <4B18BDE7.3050004@viscovery.net>

Johannes Sixt wrote:

Show 6 quoted lines

> Ramsay Jones schrieb:
>> In order to avoid the compiler warning, we use the appropriate
>> structure type names (and function names) from the msvc headers.
>> This allows us to compile with -D_USE_32BIT_TIME_T if necessary.
> 
> "if necessary"? Who defines when -D_USE_32BIT_TIME_T is necessary?

If I'm reading the msdn documentation correctly, from msvc 2005 onwards, the time_t type is 64-bits by default. In order to support "legacy apps" which break with a 64-bit time_t, the _USE_32BIT_TIME_T macro may be set in the makefile/project-file to enable the old 32-bit time_t type. (The headers contain a lot of appropriately defined "static __inline" functions which call 32- or 64-bit versions of the main time related functions; eg. mktime() will call either _mktime64() or _mktime32()).

Note that 64-bit windows does not support a 32-bit time_t.

So, the "if necessary" means: if git is broken with a 64-bit time_t and it may take some time to fix it (or we can defer fixing it for a long time). And the "Who" is: er... well "us" ;-)

Show 6 quoted lines

>> Also, I added the "&& defined(_stati64)" in the hope that it would work with
>> older msvc/sdk versions.
> 
> I think that this is an unnecessary complication and I did wonder why you
> added this extra check. Anybody doing some serious development with MS's
> tools is using VS2005 at least.

Great! So, I will drop that part of the patch.

>                                  But isn't the .vcproj file made for VS2008
> anyway?

I don't know - I don't use it. (Marius?)

Show 8 quoted lines

>> The reason for the RFC is:
>>
>>     - maybe we don't need the flexibility of compiling with/without the 32-bit
>>       time_t definition (which *works* BTW) and can revert to the original patch?
> 
> Indeed I'm wondering why we should cater for 64 bit time_t. It is an
> unnessary complication as long as MinGW gcc supports only 32 bit time_t
> and the old MSVCRT.DLL.

Ah, so you want to add -D_USE_32BIT_TIME_T to the Makefile? Do we care about 64-bit Windows?

Show 5 quoted lines

> 
>>     - I've tried to be careful not to break the MinGW build, but again I can't
>>       test it. (I will be shocked if I have ;-)
> 
> It compiles without warnings and doesn't break t/t[01]* ;)

Thanks! v2 of patch comming soon.

ATB, Ramsay Jones