{"thread":{"id":"42527","subject":"[PATCH] regex: fix a SIZE_MAX macro redefinition warning","startedAt":"2016-06-03T23:29:35Z","lastAt":"2016-06-06T18:10:48Z","messageCount":5,"participants":["Ramsay Jones","Johannes Schindelin","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"288348","messageId":"575212DF.90209@ramsayjones.plus.com","threadId":"42527","inReplyTo":null,"subject":"[PATCH] regex: fix a SIZE_MAX macro redefinition warning","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsayjones.plus.com","sentAt":"2016-06-03T23:29:35Z","receivedAt":"2016-06-03T23:29:35Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"\nSince commit 56a1a3ab (\"Silence GCC's \\\"cast of pointer to integer of a\ndifferent size\\\" warning\", 26-10-2015), sparse has been issuing a macro\nredefinition warning for the SIZE_MAX macro. However, gcc did not issue\nany such warning.\n\nAfter commit 56a1a3ab, in terms of the order of #includes and #defines,\nthe code looked something like:\n\n  $ cat -n junk.c\n       1\t#include <stddef.h>\n       2\n       3\t#define SIZE_MAX ((size_t) -1)\n       4\n       5\t#include <stdint.h>\n       6\n       7\tint main(int argc, char *argv[])\n       8\t{\n       9\t\treturn 0;\n      10\t}\n  $\n  $ gcc junk.c\n  $\n\nHowever, if you compile that file with -Wsystem-headers, then it will\nalso issue a warning. Having set -Wsystem-headers in CFLAGS, using the\nconfig.mak file, then (on cygwin):\n\n  $ make compat/regex/regex.o\n      CC compat/regex/regex.o\n  In file included from /usr/lib/gcc/x86_64-pc-cygwin/4.9.3/include/stdint.h:9:0,\n                   from compat/regex/regcomp.c:21,\n                   from compat/regex/regex.c:77:\n  /usr/include/stdint.h:362:0: warning: \"SIZE_MAX\" redefined\n   #define SIZE_MAX (__SIZE_MAX__)\n   ^\n  In file included from compat/regex/regex.c:69:0:\n  compat/regex/regex_internal.h:108:0: note: this is the location of the previous definition\n   # define SIZE_MAX ((size_t) -1)\n   ^\n  $\n\nThe compilation of the compat/regex code is somewhat unusual in that the\nregex.c file directly #includes the other c files (regcomp.c, regexec.c\nand regex_internal.c). Commit 56a1a3ab added an #include of <stdint.h>\nto the regcomp.c file, which results in the redefinition, since this is\nincluded after the regex_internal.h header. This header file contains a\n'fallback' definition for SIZE_MAX, in order to support systems which do\nnot have the <stdint.h> header (the HAVE_STDINT_H macro is not defined).\n\nIn order to suppress the warning, we remove the #include of <stdint.h>\nfrom regcomp.c and set the HAVE_STDINT_H macro, using the regex.o build\nrule within the Makefile, to ensure that <stdint.h> is #included from\nthe regex_internal.h header.\n\nSigned-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com>\n---\n\nHi Junio,\n\nAbout seven months ago, sparse started complaining but, because gcc wasn't, I\njust assumed this was a sparse bug. I put it on my sparse TODO list and pretty\nmuch forgot about it. Tonight I decided to take a quick look and was a bit\nsurprised by what I found. ;-)\n\nI spent some time worrying about the 'unconditional inclusion' of <stdint.h>\n(there used to be systems that didn't supply that header that we wanted to\nsupport), but I suspect that is now a non-issue. In any event, I don't\nthink this patch makes things any worse.\n\n[Just a flavour of the rabbit holes I went down; HP-NONSTOP seems to supply\nboth <inttypes.h> and <stdint.h> but it doesn't supply the intptr_t and\nuintptr_t types (ie it defines NO_INTPTR_T). So, I checked the C99 and C11\nstandards and, blow me down, but the standard states that those types are\n_optional_. (C99 7.18.1.4) :-D ]\n\nATB,\nRamsay Jones\n\n Makefile               | 2 +-\n compat/regex/regcomp.c | 2 --\n 2 files changed, 1 insertion(+), 3 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex 0d59718..3f6c70a 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -1987,7 +1987,7 @@ endif\n \n ifdef NO_REGEX\n compat/regex/regex.sp compat/regex/regex.o: EXTRA_CPPFLAGS = \\\n-\t-DGAWK -DNO_MBSUPPORT\n+\t-DGAWK -DNO_MBSUPPORT -DHAVE_STDINT_H\n endif\n \n ifdef USE_NED_ALLOCATOR\ndiff --git a/compat/regex/regcomp.c b/compat/regex/regcomp.c\nindex fba5986..d8bde06 100644\n--- a/compat/regex/regcomp.c\n+++ b/compat/regex/regcomp.c\n@@ -18,8 +18,6 @@\n    Software Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA\n    02110-1301 USA.  */\n \n-#include <stdint.h>\n-\n static reg_errcode_t re_compile_internal (regex_t *preg, const char * pattern,\n \t\t\t\t\t  size_t length, reg_syntax_t syntax);\n static void re_compile_fastmap_iter (regex_t *bufp,\n-- \n2.8.0\n"},{"id":"288390","messageId":"alpine.DEB.2.20.1606050815360.4250@virtualbox","threadId":"42527","inReplyTo":"575212DF.90209@ramsayjones.plus.com","subject":"Re: [PATCH] regex: fix a SIZE_MAX macro redefinition warning","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-06-05T07:15:27Z","receivedAt":"2016-06-05T07:15:27Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Ramsay,\n\nthanks for working on this!\n\nOn Sat, 4 Jun 2016, Ramsay Jones wrote:\n\n> diff --git a/Makefile b/Makefile\n> index 0d59718..3f6c70a 100644\n> --- a/Makefile\n> +++ b/Makefile\n> @@ -1987,7 +1987,7 @@ endif\n>  \n>  ifdef NO_REGEX\n>  compat/regex/regex.sp compat/regex/regex.o: EXTRA_CPPFLAGS = \\\n> -\t-DGAWK -DNO_MBSUPPORT\n> +\t-DGAWK -DNO_MBSUPPORT -DHAVE_STDINT_H\n\nMaybe a comment here, something like \"the fallback regex implementation\n*requires* stdint.h\"?\n\nOther than that, I think this patch is an improvement.\n\nThanks again,\nDscho\n"},{"id":"288394","messageId":"57542A85.3040206@ramsayjones.plus.com","threadId":"42527","inReplyTo":"alpine.DEB.2.20.1606050815360.4250@virtualbox","subject":"Re: [PATCH] regex: fix a SIZE_MAX macro redefinition warning","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsayjones.plus.com","sentAt":"2016-06-05T13:35:01Z","receivedAt":"2016-06-05T13:35:01Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"\n\nOn 05/06/16 08:15, Johannes Schindelin wrote:\n> Hi Ramsay,\n> \n> thanks for working on this!\n> \n> On Sat, 4 Jun 2016, Ramsay Jones wrote:\n> \n>> diff --git a/Makefile b/Makefile\n>> index 0d59718..3f6c70a 100644\n>> --- a/Makefile\n>> +++ b/Makefile\n>> @@ -1987,7 +1987,7 @@ endif\n>>  \n>>  ifdef NO_REGEX\n>>  compat/regex/regex.sp compat/regex/regex.o: EXTRA_CPPFLAGS = \\\n>> -\t-DGAWK -DNO_MBSUPPORT\n>> +\t-DGAWK -DNO_MBSUPPORT -DHAVE_STDINT_H\n> \n> Maybe a comment here, something like \"the fallback regex implementation\n> *requires* stdint.h\"?\n\nThe original version of this patch looked like this:\n\n  diff --git a/compat/regex/regcomp.c b/compat/regex/regcomp.c\n  index fba5986..d8bde06 100644\n  --- a/compat/regex/regcomp.c\n  +++ b/compat/regex/regcomp.c\n  @@ -18,8 +18,6 @@\n      Software Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA\n      02110-1301 USA.  */\n   \n  -#include <stdint.h>\n  -\n   static reg_errcode_t re_compile_internal (regex_t *preg, const char * pattern,\n   \t\t\t\t\t  size_t length, reg_syntax_t syntax);\n   static void re_compile_fastmap_iter (regex_t *bufp,\n  diff --git a/compat/regex/regex.c b/compat/regex/regex.c\n  index 6aaae00..5cb23e5 100644\n  --- a/compat/regex/regex.c\n  +++ b/compat/regex/regex.c\n  @@ -60,6 +60,7 @@\n      GNU regex allows.  Include it before <regex.h>, which correctly\n      #undefs RE_DUP_MAX and sets it to the right value.  */\n   #include <limits.h>\n  +#include <stdint.h>\n   \n   #ifdef GAWK\n   #undef alloca\n\nSo, just move the unconditional inclusion to the start of the compilation\nunit root file, before the #include of the regex_internal.h header.\n\nIn some ways this is a better fix, because it makes it clear that, currently,\nthe compat/regex code requires <stdint.h>. This would remove the need for\nsuch a comment.\n\nThis effectively makes the conditional inclusion of <stdint.h>, and the SIZE_MAX\nfallback, in regex_internal.h dead code. (The C99 standard _requires_ the\ndefinition of SIZE_MAX in <stdint.h>, thankfully! ;-). So, I was tempted to\nremove them as part of the patch. However, I also wanted to minimize the changes\nto the regex code, just in case we ever wanted to re-import a newer version\nfrom upstream. Setting HAVE_STDINT_H seemed like a good solution, but maybe the\nfirst patch would be more honest?\n\nAs I said earlier, I was a little concerned about the 'unconditional' aspect of\nthe inclusion of <stdint.h>. At one time we wanted to support systems that didn't\nhave <stdint.h> (or didn't have <inttypes.h> but did have <stdint.h>). However,\nit has been several months and we have not heard anyone scream, so ...\n\nIt is slightly amusing that the reason you #included <stdint.h> was to get the\ndefinition of 'intptr_t' and the C standard states that this type is optional.\nIn practice, I suspect that the number of platforms which do not define 'intptr_t'\nand 'uintptr_t' in the <stdint.h> header is rather small.\n\nHaving said that ... If I'm reading the code/config correctly, HP-NONSTOP would\nbe failing to compile at the moment. (Although it has <stdint.h>, it does not\ndefine 'intptr_t' - ie it defines 'NO_REGEX = YesPlease' and 'NO_INTPTR_T = \\\nUnfortunatelyYes').\n\n> Other than that, I think this patch is an improvement.\n\nThanks. What do you think of replacing it with the original patch (above)?\n\nATB,\nRamsay Jones\n"},{"id":"288406","messageId":"alpine.DEB.2.20.1606060856230.28610@virtualbox","threadId":"42527","inReplyTo":"57542A85.3040206@ramsayjones.plus.com","subject":"Re: [PATCH] regex: fix a SIZE_MAX macro redefinition warning","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-06-06T06:57:10Z","receivedAt":"2016-06-06T06:57:10Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Ramsay,\n\nOn Sun, 5 Jun 2016, Ramsay Jones wrote:\n\n> > Other than that, I think this patch is an improvement.\n> \n> Thanks. What do you think of replacing it with the original patch\n> (above)?\n\nThat would work for me, too.\n\nThanks,\nDscho\n"},{"id":"288468","messageId":"xmqqd1nuxk5z.fsf@gitster.mtv.corp.google.com","threadId":"42527","inReplyTo":"57542A85.3040206@ramsayjones.plus.com","subject":"Re: [PATCH] regex: fix a SIZE_MAX macro redefinition warning","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-06-06T18:10:48Z","receivedAt":"2016-06-06T18:10:48Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ramsay Jones <ramsay@ramsayjones.plus.com> writes:\n\n> The original version of this patch looked like this:\n> ...\n> So, just move the unconditional inclusion to the start of the compilation\n> unit root file, before the #include of the regex_internal.h header.\n>\n> In some ways this is a better fix, because it makes it clear that, currently,\n> the compat/regex code requires <stdint.h>. This would remove the need for\n> such a comment.\n\nSounds sensible to me too.\n"}]}