[PATCH 04/14] msvc: Fix macro redefinition warnings

[PATCH 04/14] msvc: Fix macro redefinition warnings

[Ramsay Jones]

Commit 4091bfc (MinGW: Add missing file mode bit defines,
28-12-2009) causes the msvc build to issue many additional
(currently 1008) macro redefinition warnings. The warnings
relate to the S_IRUSR, S_IWUSR, S_IXUSR and S_IRWXU macros.

In order to fix the warnings, we simply remove the offending
macro definitions which, for both msvc and MinGW, are not
required.

Signed-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
---
 compat/mingw.h |    6 ------
 1 files changed, 0 insertions(+), 6 deletions(-)

diff --git a/compat/mingw.h b/compat/mingw.h
index 99a7467..da316dc 100644
--- a/compat/mingw.h
+++ b/compat/mingw.h
@@ -14,12 +14,6 @@ typedef int socklen_t;
 #define S_ISLNK(x) (((x) & S_IFMT) == S_IFLNK)
 #define S_ISSOCK(x) 0
 
-#ifndef _STAT_H_
-#define S_IRUSR 0
-#define S_IWUSR 0
-#define S_IXUSR 0
-#define S_IRWXU (S_IRUSR | S_IWUSR | S_IXUSR)
-#endif
 #define S_IRGRP 0
 #define S_IWGRP 0
 #define S_IXGRP 0
-- 
1.7.3

Re: [PATCH 04/14] msvc: Fix macro redefinition warnings

[Johannes Sixt]

On Samstag, 4. Dezember 2010, Ramsay Jones wrote:
> --- a/compat/mingw.h
> +++ b/compat/mingw.h
> @@ -14,12 +14,6 @@ typedef int socklen_t;
>  #define S_ISLNK(x) (((x) & S_IFMT) == S_IFLNK)
>  #define S_ISSOCK(x) 0
>
> -#ifndef _STAT_H_

Instead of removing the macros, wouldn't we be much safer with just

#ifndef S_IWUSR

? I suggest S_IWUSR because it is used inside #ifdef WIN32 brackets in 
test-chmtime.c and seems to be more important than the others :-)

> -#define S_IRUSR 0
> -#define S_IWUSR 0
> -#define S_IXUSR 0
> -#define S_IRWXU (S_IRUSR | S_IWUSR | S_IXUSR)
> -#endif
>  #define S_IRGRP 0
>  #define S_IWGRP 0
>  #define S_IXGRP 0

-- Hannes

Re: [PATCH 04/14] msvc: Fix macro redefinition warnings

[Ramsay Jones]

Johannes Sixt wrote:
> On Samstag, 4. Dezember 2010, Ramsay Jones wrote:
>> --- a/compat/mingw.h
>> +++ b/compat/mingw.h
>> @@ -14,12 +14,6 @@ typedef int socklen_t;
>>  #define S_ISLNK(x) (((x) & S_IFMT) == S_IFLNK)
>>  #define S_ISSOCK(x) 0
>>
>> -#ifndef _STAT_H_
> 
> Instead of removing the macros, wouldn't we be much safer with just
> 
> #ifndef S_IWUSR
> 
> ? ...

Er... no.

Commit 4091bfc (which added these macros) does not provide any motivation
for the change, and I'm having a hard time trying to imagine a useful
purpose for this part of the commit. (I'm not saying there isn't one - just
that I can't see it :-P )

On MinGW, the <sys/stat.h> header is always included prior to this header,
so the _STAT_H_ include guard is always defined, so these macros will
never be defined (which is a *good* thing; have you looked at the definitions).
Trying to use compat/mingw.h without having first included <sys/stat.h> is
not going to work!

Also, note that the include guard for the msvc <sys/stat.h> file is _INC_STAT.
So, on msvc, including <sys/stat.h> does not suppress these macro definitions
(Not that it actually matters here, because it doesn't define these symbols
anyway!). Which is why msvc issues these macro redefinition warnings (they
conflict with the definitions in compat/vcbuild/include/unistd.h). We most
definitely don't want to use the macros in compat/mingw.h on msvc. (They are
positively *wrong*)

[Hmmm, I've just noticed that the msvc compat header is missing a definition of
the _S_IRWXU macro!]

So, once again, I see no reason to keep them ... Unless you know otherwise.

ATB,
Ramsay Jones

Re: [PATCH 04/14] msvc: Fix macro redefinition warnings

[Sebastian Schuberth]

On Wed, Dec 8, 2010 at 01:05, Ramsay Jones <ramsay@ramsay1.demon.co.uk> wrote:

> Johannes Sixt wrote:

>> Instead of removing the macros, wouldn't we be much safer with just
>>
>> #ifndef S_IWUSR
>>
>> ? ...
>
> Er... no.
>
> Commit 4091bfc (which added these macros) does not provide any motivation
> for the change, and I'm having a hard time trying to imagine a useful
> purpose for this part of the commit. (I'm not saying there isn't one - just
> that I can't see it :-P )

Sorry for not finding the time to respond to the thread in [1] about
two months ago where this issue about my commit was first raised.
While it's true that my commit message does not contain any detailed
information about its motivation, it says the defines were "missing",
suggesting a compile error. Indeed, I remember that back then my
msysgit working tree did not compile with MSVC if I didn't have these
defines (and I vaguely remember that this was caused by MSVC using
different a header file than MinGW, or in a different order, or
something similar).

However, I'm not able to reproduce this anymore. I checked out
4091bfc^ and 4091bfc, and both compile file with MSVC for me now, the
latter just giving a lot of the mentioned macro redefinition warnings.
Maybe this was caused my me using older MSVC project files with a
newer code base ... I probably should have run
contrib/buildsystems/generate again.

After defining LF_FACESIZE and TMPF_TRUETYPE in winansi.c, and
INTMAX_MAX in git-compat-util.h, I was also able to compile the
v1.7.3.2.msysgit.0 tag with MSVC. If I revert 4091bfc on top of it, it
still compiles fine for me.

> So, once again, I see no reason to keep them ... Unless you know otherwise.

I agree to remove the lines and vote in favor of Ramsay's patch. Feel
free to add me as Signed-off-by or Acked-by.

[1] http://thread.gmane.org/gmane.comp.version-control.git/158144/focus=158409

-- 
Sebastian Schuberth

Re: [PATCH 04/14] msvc: Fix macro redefinition warnings

[Johannes Sixt]

On Mittwoch, 8. Dezember 2010, Sebastian Schuberth wrote:
> Indeed, I remember that back then my
> msysgit working tree did not compile with MSVC if I didn't have these
> defines (and I vaguely remember that this was caused by MSVC using
> different a header file than MinGW, or in a different order, or
> something similar).
...
> After defining LF_FACESIZE and TMPF_TRUETYPE in winansi.c, and
> INTMAX_MAX in git-compat-util.h, I was also able to compile the
> v1.7.3.2.msysgit.0 tag with MSVC. If I revert 4091bfc on top of it, it
> still compiles fine for me.
>
> > So, once again, I see no reason to keep them ... Unless you know
> > otherwise.
>
> I agree to remove the lines and vote in favor of Ramsay's patch. Feel
> free to add me as Signed-off-by or Acked-by.

Under these circumstances, I agree as well with Ramsay's patch.

-- Hannes