{"thread":{"id":"58097","subject":"undefined behavior in builtin/am.c","startedAt":"2022-07-01T17:03:24Z","lastAt":"2022-07-01T18:58:33Z","messageCount":4,"participants":["Roland Illig","Jeff King","Phillip Wood"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"458393","messageId":"130c3636-b978-1600-df53-6a38c3414a88@gmx.de","threadId":"58097","inReplyTo":null,"subject":"undefined behavior in builtin/am.c","fromName":"Roland Illig","fromEmail":"roland.illig@gmx.de","sentAt":"2022-07-01T17:03:18Z","receivedAt":"2022-07-01T17:03:24Z","isPatch":false,"sender":{"key":"roland.illig@gmx.de","avatar":null},"body":"Hi,\n\nbuiltin/am.c says:\n > static int str_isspace(const char *str)\n > {\n > \tfor (; *str; str++)\n > \t\tif (!isspace(*str))\n > \t\t\treturn 0;\n >\n > \treturn 1;\n > }\n\nThe macro 'isspace' must only be called with an integer representable as\nan 'unsigned char', or with the value of the macro EOF.\n\nOn platforms where plain 'char' is a signed integer type, any character\nwhose value is negative invokes undefined behavior (except for the one\ncharacter that by coincidence has the same value as the macro EOF).\n\nTo fix this, write '!isspace((unsigned char)*str)' instead.\n\nI have no idea how to trigger this part of the code but for someone who\nknows this part of Git, it should be easy. Depending on the platform,\nthis kind of error may be silently ignored or crash the program, as\nalways with undefined behavior.\n\nRoland\n"},{"id":"458396","messageId":"Yr80s8VP8ECCXKd+@coredump.intra.peff.net","threadId":"58097","inReplyTo":"130c3636-b978-1600-df53-6a38c3414a88@gmx.de","subject":"Re: undefined behavior in builtin/am.c","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2022-07-01T17:53:55Z","receivedAt":"2022-07-01T17:54:00Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Jul 01, 2022 at 07:03:18PM +0200, Roland Illig wrote:\n\n> The macro 'isspace' must only be called with an integer representable as\n> an 'unsigned char', or with the value of the macro EOF.\n> \n> On platforms where plain 'char' is a signed integer type, any character\n> whose value is negative invokes undefined behavior (except for the one\n> character that by coincidence has the same value as the macro EOF).\n> \n> To fix this, write '!isspace((unsigned char)*str)' instead.\n> \n> I have no idea how to trigger this part of the code but for someone who\n> knows this part of Git, it should be easy. Depending on the platform,\n> this kind of error may be silently ignored or crash the program, as\n> always with undefined behavior.\n\nWe don't use the system isspace(), but instead our own macro wrappers in\ngit-compat-util.h. They do the cast to unsigned char themselves.\n\nI won't be surprised if re-defining a system name as a macro is also\ntechnically undefined behavior, but I don't think we've found a system\nthat has a problem with it in the past 17 years. :)\n\n-Peff\n"},{"id":"458397","messageId":"d717324b-d7dc-fc5c-7bd7-2b74c4dfb644@gmail.com","threadId":"58097","inReplyTo":"130c3636-b978-1600-df53-6a38c3414a88@gmx.de","subject":"Re: undefined behavior in builtin/am.c","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2022-07-01T17:54:29Z","receivedAt":"2022-07-01T17:54:36Z","isPatch":false,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Roland\n\nOn 01/07/2022 18:03, Roland Illig wrote:\n> Hi,\n> \n> builtin/am.c says:\n>  > static int str_isspace(const char *str)\n>  > {\n>  >     for (; *str; str++)\n>  >         if (!isspace(*str))\n>  >             return 0;\n>  >\n>  >     return 1;\n>  > }\n> \n> The macro 'isspace' must only be called with an integer representable as\n> an 'unsigned char', or with the value of the macro EOF.\n\nThanks for reporting this. Git uses its own version of isspace() (see \ngit-compat-util.h & ctype.c) which does not suffer from this limitation \nas it casts its argument to an unsigned char.\n\nBest Wishes\n\nPhillip\n\n> On platforms where plain 'char' is a signed integer type, any character\n> whose value is negative invokes undefined behavior (except for the one\n> character that by coincidence has the same value as the macro EOF).\n> \n> To fix this, write '!isspace((unsigned char)*str)' instead.\n> \n> I have no idea how to trigger this part of the code but for someone who\n> knows this part of Git, it should be easy. Depending on the platform,\n> this kind of error may be silently ignored or crash the program, as\n> always with undefined behavior.\n> \n> Roland\n\n"},{"id":"458407","messageId":"647f50a7-4350-fe04-fb84-b79417d98455@gmx.de","threadId":"58097","inReplyTo":"Yr80s8VP8ECCXKd+@coredump.intra.peff.net","subject":"Re: undefined behavior in builtin/am.c","fromName":"Roland Illig","fromEmail":"roland.illig@gmx.de","sentAt":"2022-07-01T18:58:25Z","receivedAt":"2022-07-01T18:58:33Z","isPatch":false,"sender":{"key":"roland.illig@gmx.de","avatar":null},"body":"Am 01.07.2022 um 19:53 schrieb Jeff King:\n> On Fri, Jul 01, 2022 at 07:03:18PM +0200, Roland Illig wrote:\n>\n>> The macro 'isspace' must only be called with an integer representable as\n>> an 'unsigned char', or with the value of the macro EOF.\n>>\n>> On platforms where plain 'char' is a signed integer type, any character\n>> whose value is negative invokes undefined behavior (except for the one\n>> character that by coincidence has the same value as the macro EOF).\n>>\n>> To fix this, write '!isspace((unsigned char)*str)' instead.\n>>\n>> I have no idea how to trigger this part of the code but for someone who\n>> knows this part of Git, it should be easy. Depending on the platform,\n>> this kind of error may be silently ignored or crash the program, as\n>> always with undefined behavior.\n>\n> We don't use the system isspace(), but instead our own macro wrappers in\n> git-compat-util.h. They do the cast to unsigned char themselves.\n\nThanks for the explanation, good to know that there's no problem. :)\n\nRoland\n"}]}