{"thread":{"id":"25962","subject":"[PATCH 04/14] msvc: Fix macro redefinition warnings","startedAt":"2010-12-04T19:00:51Z","lastAt":"2010-12-08T19:51:46Z","messageCount":5,"participants":["Ramsay Jones","Johannes Sixt","Sebastian Schuberth"],"isPatch":true,"patchVersion":1,"patchTotal":14},"messages":[{"id":"157321","messageId":"4CFA8FE3.1050009@ramsay1.demon.co.uk","threadId":"25962","inReplyTo":null,"subject":"[PATCH 04/14] msvc: Fix macro redefinition warnings","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsay1.demon.co.uk","sentAt":"2010-12-04T19:00:51Z","receivedAt":"2010-12-04T19:00:51Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"\nCommit 4091bfc (MinGW: Add missing file mode bit defines,\n28-12-2009) causes the msvc build to issue many additional\n(currently 1008) macro redefinition warnings. The warnings\nrelate to the S_IRUSR, S_IWUSR, S_IXUSR and S_IRWXU macros.\n\nIn order to fix the warnings, we simply remove the offending\nmacro definitions which, for both msvc and MinGW, are not\nrequired.\n\nSigned-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>\n---\n compat/mingw.h |    6 ------\n 1 files changed, 0 insertions(+), 6 deletions(-)\n\ndiff --git a/compat/mingw.h b/compat/mingw.h\nindex 99a7467..da316dc 100644\n--- a/compat/mingw.h\n+++ b/compat/mingw.h\n@@ -14,12 +14,6 @@ typedef int socklen_t;\n #define S_ISLNK(x) (((x) & S_IFMT) == S_IFLNK)\n #define S_ISSOCK(x) 0\n \n-#ifndef _STAT_H_\n-#define S_IRUSR 0\n-#define S_IWUSR 0\n-#define S_IXUSR 0\n-#define S_IRWXU (S_IRUSR | S_IWUSR | S_IXUSR)\n-#endif\n #define S_IRGRP 0\n #define S_IWGRP 0\n #define S_IXGRP 0\n-- \n1.7.3\n"},{"id":"157323","messageId":"201012042204.52002.j6t@kdbg.org","threadId":"25962","inReplyTo":"4CFA8FE3.1050009@ramsay1.demon.co.uk","subject":"Re: [PATCH 04/14] msvc: Fix macro redefinition warnings","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2010-12-04T21:04:51Z","receivedAt":"2010-12-04T21:04:51Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"On Samstag, 4. Dezember 2010, Ramsay Jones wrote:\n> --- a/compat/mingw.h\n> +++ b/compat/mingw.h\n> @@ -14,12 +14,6 @@ typedef int socklen_t;\n>  #define S_ISLNK(x) (((x) & S_IFMT) == S_IFLNK)\n>  #define S_ISSOCK(x) 0\n>\n> -#ifndef _STAT_H_\n\nInstead of removing the macros, wouldn't we be much safer with just\n\n#ifndef S_IWUSR\n\n? I suggest S_IWUSR because it is used inside #ifdef WIN32 brackets in \ntest-chmtime.c and seems to be more important than the others :-)\n\n> -#define S_IRUSR 0\n> -#define S_IWUSR 0\n> -#define S_IXUSR 0\n> -#define S_IRWXU (S_IRUSR | S_IWUSR | S_IXUSR)\n> -#endif\n>  #define S_IRGRP 0\n>  #define S_IWGRP 0\n>  #define S_IXGRP 0\n\n-- Hannes\n"},{"id":"157560","messageId":"4CFECBB8.2070900@ramsay1.demon.co.uk","threadId":"25962","inReplyTo":"201012042204.52002.j6t@kdbg.org","subject":"Re: [PATCH 04/14] msvc: Fix macro redefinition warnings","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsay1.demon.co.uk","sentAt":"2010-12-08T00:05:12Z","receivedAt":"2010-12-08T00:05:12Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"Johannes Sixt wrote:\n> On Samstag, 4. Dezember 2010, Ramsay Jones wrote:\n>> --- a/compat/mingw.h\n>> +++ b/compat/mingw.h\n>> @@ -14,12 +14,6 @@ typedef int socklen_t;\n>>  #define S_ISLNK(x) (((x) & S_IFMT) == S_IFLNK)\n>>  #define S_ISSOCK(x) 0\n>>\n>> -#ifndef _STAT_H_\n> \n> Instead of removing the macros, wouldn't we be much safer with just\n> \n> #ifndef S_IWUSR\n> \n> ? ...\n\nEr... no.\n\nCommit 4091bfc (which added these macros) does not provide any motivation\nfor the change, and I'm having a hard time trying to imagine a useful\npurpose for this part of the commit. (I'm not saying there isn't one - just\nthat I can't see it :-P )\n\nOn MinGW, the <sys/stat.h> header is always included prior to this header,\nso the _STAT_H_ include guard is always defined, so these macros will\nnever be defined (which is a *good* thing; have you looked at the definitions).\nTrying to use compat/mingw.h without having first included <sys/stat.h> is\nnot going to work!\n\nAlso, note that the include guard for the msvc <sys/stat.h> file is _INC_STAT.\nSo, on msvc, including <sys/stat.h> does not suppress these macro definitions\n(Not that it actually matters here, because it doesn't define these symbols\nanyway!). Which is why msvc issues these macro redefinition warnings (they\nconflict with the definitions in compat/vcbuild/include/unistd.h). We most\ndefinitely don't want to use the macros in compat/mingw.h on msvc. (They are\npositively *wrong*)\n\n[Hmmm, I've just noticed that the msvc compat header is missing a definition of\nthe _S_IRWXU macro!]\n\nSo, once again, I see no reason to keep them ... Unless you know otherwise.\n\nATB,\nRamsay Jones\n"},{"id":"157592","messageId":"AANLkTimpj31CSzaxH4ZuYdADvtV4KSwfk04eGSRYSLFH@mail.gmail.com","threadId":"25962","inReplyTo":"4CFECBB8.2070900@ramsay1.demon.co.uk","subject":"Re: [PATCH 04/14] msvc: Fix macro redefinition warnings","fromName":"Sebastian Schuberth","fromEmail":"sschuberth@gmail.com","sentAt":"2010-12-08T10:32:40Z","receivedAt":"2010-12-08T10:32:40Z","isPatch":true,"sender":{"key":"sschuberth@gmail.com","avatar":"https://avatars.githubusercontent.com/u/349154?v=4"},"body":"On Wed, Dec 8, 2010 at 01:05, Ramsay Jones <ramsay@ramsay1.demon.co.uk> wrote:\n\n> Johannes Sixt wrote:\n\n>> Instead of removing the macros, wouldn't we be much safer with just\n>>\n>> #ifndef S_IWUSR\n>>\n>> ? ...\n>\n> Er... no.\n>\n> Commit 4091bfc (which added these macros) does not provide any motivation\n> for the change, and I'm having a hard time trying to imagine a useful\n> purpose for this part of the commit. (I'm not saying there isn't one - just\n> that I can't see it :-P )\n\nSorry for not finding the time to respond to the thread in [1] about\ntwo months ago where this issue about my commit was first raised.\nWhile it's true that my commit message does not contain any detailed\ninformation about its motivation, it says the defines were \"missing\",\nsuggesting a compile error. Indeed, I remember that back then my\nmsysgit working tree did not compile with MSVC if I didn't have these\ndefines (and I vaguely remember that this was caused by MSVC using\ndifferent a header file than MinGW, or in a different order, or\nsomething similar).\n\nHowever, I'm not able to reproduce this anymore. I checked out\n4091bfc^ and 4091bfc, and both compile file with MSVC for me now, the\nlatter just giving a lot of the mentioned macro redefinition warnings.\nMaybe this was caused my me using older MSVC project files with a\nnewer code base ... I probably should have run\ncontrib/buildsystems/generate again.\n\nAfter defining LF_FACESIZE and TMPF_TRUETYPE in winansi.c, and\nINTMAX_MAX in git-compat-util.h, I was also able to compile the\nv1.7.3.2.msysgit.0 tag with MSVC. If I revert 4091bfc on top of it, it\nstill compiles fine for me.\n\n> So, once again, I see no reason to keep them ... Unless you know otherwise.\n\nI agree to remove the lines and vote in favor of Ramsay's patch. Feel\nfree to add me as Signed-off-by or Acked-by.\n\n[1] http://thread.gmane.org/gmane.comp.version-control.git/158144/focus=158409\n\n-- \nSebastian Schuberth\n"},{"id":"157623","messageId":"201012082051.46644.j6t@kdbg.org","threadId":"25962","inReplyTo":"AANLkTimpj31CSzaxH4ZuYdADvtV4KSwfk04eGSRYSLFH@mail.gmail.com","subject":"Re: [PATCH 04/14] msvc: Fix macro redefinition warnings","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2010-12-08T19:51:46Z","receivedAt":"2010-12-08T19:51:46Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"On Mittwoch, 8. Dezember 2010, Sebastian Schuberth wrote:\n> Indeed, I remember that back then my\n> msysgit working tree did not compile with MSVC if I didn't have these\n> defines (and I vaguely remember that this was caused by MSVC using\n> different a header file than MinGW, or in a different order, or\n> something similar).\n...\n> After defining LF_FACESIZE and TMPF_TRUETYPE in winansi.c, and\n> INTMAX_MAX in git-compat-util.h, I was also able to compile the\n> v1.7.3.2.msysgit.0 tag with MSVC. If I revert 4091bfc on top of it, it\n> still compiles fine for me.\n>\n> > So, once again, I see no reason to keep them ... Unless you know\n> > otherwise.\n>\n> I agree to remove the lines and vote in favor of Ramsay's patch. Feel\n> free to add me as Signed-off-by or Acked-by.\n\nUnder these circumstances, I agree as well with Ramsay's patch.\n\n-- Hannes\n"}]}