{"thread":{"id":"48839","subject":"[RFC PATCH 0/6] Compile cleanly in pedantic mode","startedAt":"2018-07-08T14:43:55Z","lastAt":"2018-07-12T15:40:26Z","messageCount":39,"participants":["Beat Bolli","Eric Sunshine","Philip Oakley","ig@drbeat.li","Johannes Schindelin","Junio C Hamano","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":6},"messages":[{"id":"351867","messageId":"20180708144342.11922-1-dev+git@drbeat.li","threadId":"48839","inReplyTo":null,"subject":"[RFC PATCH 0/6] Compile cleanly in pedantic mode","fromName":"Beat Bolli","fromEmail":"dev+git@drbeat.li","sentAt":"2018-07-08T14:43:36Z","receivedAt":"2018-07-08T14:43:55Z","isPatch":true,"sender":{"key":"dev+git@drbeat.li","avatar":"https://avatars.githubusercontent.com/u/21444?v=4"},"body":"While developing 6aaded550 (\"builtin/config: work around an unsized\narray forward declaration\", 2018-07-05), I have compiled Git with\nCFLAGS=\"-std=c99 -pedantic\".\n\nThis is an RFC patch series that fixes a few compiler warnings when\ncompiling with these options, always assuming that this is a worthwile\ngoal.\n\nNote that all warnings were produced by -pedantic; the C99 standard\noption by itself didn't cause any of them.\n\nThe warnings were:\n\n1) Char arrays initialized from a parenthesized string.\n\n        Suppressed by defining USE_PARENS_AROUND_GETTEXT_N to 0\n        globally. This was done just to keep the amount of warnings\n        manageable; this series leaves that knob alone. The advantage of\n        not mistakenly concatenating two translated strings is greater.\n\n2) connect.h, refs/refs-internal.h: Forward reference to an enum.\n\n        Added two #includes that define the enums. This was already\n        (inconclusively) talked about in [0].\n\n3) convert.c: Invalid escape sequence \"\\e\".\n\n        Replaced with \"\\033\".\n\n4) seqencer.c: Empty statements at top level.\n\n        Removed the extra semicolons.\n\n5) string-list.c: Forbidden to cast from void * to a function pointer and\n   vice versa.\n\n        Encapsulated the function pointer in a context struct. This is\n        controversial because it has a performance impact, namely one\n        additional pointer dereference per string comparison. An\n        alternative might be to use multiple casts via intptr_t. But\n        I'm not sure if this is worth the trouble.\n\n6) utf8.c: overflow of char values.\n\n        Use unsigned char for the BOM constants.\n\nThis series has patches for 2) to 6).\n\nRegards,\nBeat\n\n[0] https://public-inbox.org/git/53ab8626-f862-a732-b369-abeab69a468f@ramsayjones.plus.com/T/\n"},{"id":"351868","messageId":"20180708144342.11922-5-dev+git@drbeat.li","threadId":"48839","inReplyTo":"20180708144342.11922-1-dev+git@drbeat.li","subject":"[RFC PATCH 4/6] sequencer.c: avoid empty statements at top level","fromName":"Beat Bolli","fromEmail":"dev+git@drbeat.li","sentAt":"2018-07-08T14:43:40Z","receivedAt":"2018-07-08T14:44:12Z","isPatch":true,"sender":{"key":"dev+git@drbeat.li","avatar":"https://avatars.githubusercontent.com/u/21444?v=4"},"body":"The marco GIT_PATH_FUNC expands to a complete statement including the\nsemicolon. Remove two extra trailing semicolons.\n\nSigned-off-by: Beat Bolli <dev+git@drbeat.li>\n---\n sequencer.c | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/sequencer.c b/sequencer.c\nindex 5354d4d51e..66e7073995 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -62,12 +62,12 @@ static GIT_PATH_FUNC(rebase_path_done, \"rebase-merge/done\")\n  * The file to keep track of how many commands were already processed (e.g.\n  * for the prompt).\n  */\n-static GIT_PATH_FUNC(rebase_path_msgnum, \"rebase-merge/msgnum\");\n+static GIT_PATH_FUNC(rebase_path_msgnum, \"rebase-merge/msgnum\")\n /*\n  * The file to keep track of how many commands are to be processed in total\n  * (e.g. for the prompt).\n  */\n-static GIT_PATH_FUNC(rebase_path_msgtotal, \"rebase-merge/end\");\n+static GIT_PATH_FUNC(rebase_path_msgtotal, \"rebase-merge/end\")\n /*\n  * The commit message that is planned to be used for any changes that\n  * need to be committed following a user interaction.\n-- \n2.15.0.rc1.299.gda03b47c3\n\n"},{"id":"351869","messageId":"20180708144342.11922-3-dev+git@drbeat.li","threadId":"48839","inReplyTo":"20180708144342.11922-1-dev+git@drbeat.li","subject":"[RFC PATCH 2/6] refs/refs-internal.h: avoid forward declaration of an enum","fromName":"Beat Bolli","fromEmail":"dev+git@drbeat.li","sentAt":"2018-07-08T14:43:38Z","receivedAt":"2018-07-08T14:44:14Z","isPatch":true,"sender":{"key":"dev+git@drbeat.li","avatar":"https://avatars.githubusercontent.com/u/21444?v=4"},"body":"Include iterator.h to define enum iterator_selection.\n\nSigned-off-by: Beat Bolli <dev+git@drbeat.li>\n---\n refs/refs-internal.h | 2 ++\n 1 file changed, 2 insertions(+)\n\ndiff --git a/refs/refs-internal.h b/refs/refs-internal.h\nindex dd834314bd..a78b5cb803 100644\n--- a/refs/refs-internal.h\n+++ b/refs/refs-internal.h\n@@ -1,6 +1,8 @@\n #ifndef REFS_REFS_INTERNAL_H\n #define REFS_REFS_INTERNAL_H\n \n+#include \"iterator.h\"   /* for enum iterator_selection */\n+\n /*\n  * Data structures and functions for the internal use of the refs\n  * module. Code outside of the refs module should use only the public\n-- \n2.15.0.rc1.299.gda03b47c3\n\n"},{"id":"351870","messageId":"20180708144342.11922-4-dev+git@drbeat.li","threadId":"48839","inReplyTo":"20180708144342.11922-1-dev+git@drbeat.li","subject":"[RFC PATCH 3/6] convert.c: replace \"\\e\" escapes with \"\\033\".","fromName":"Beat Bolli","fromEmail":"dev+git@drbeat.li","sentAt":"2018-07-08T14:43:39Z","receivedAt":"2018-07-08T14:44:15Z","isPatch":true,"sender":{"key":"dev+git@drbeat.li","avatar":"https://avatars.githubusercontent.com/u/21444?v=4"},"body":"The \"\\e\" escape is not defined in ISO C.\n\nWhile on this line, add a missing space after the comma.\n\nSigned-off-by: Beat Bolli <dev+git@drbeat.li>\n---\n convert.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/convert.c b/convert.c\nindex 64d0d30e08..edebb946f5 100644\n--- a/convert.c\n+++ b/convert.c\n@@ -334,7 +334,7 @@ static void trace_encoding(const char *context, const char *path,\n \tstrbuf_addf(&trace, \"%s (%s, considered %s):\\n\", context, path, encoding);\n \tfor (i = 0; i < len && buf; ++i) {\n \t\tstrbuf_addf(\n-\t\t\t&trace,\"| \\e[2m%2i:\\e[0m %2x \\e[2m%c\\e[0m%c\",\n+\t\t\t&trace, \"| \\033[2m%2i:\\033[0m %2x \\033[2m%c\\033[0m%c\",\n \t\t\ti,\n \t\t\t(unsigned char) buf[i],\n \t\t\t(buf[i] > 32 && buf[i] < 127 ? buf[i] : ' '),\n-- \n2.15.0.rc1.299.gda03b47c3\n\n"},{"id":"351871","messageId":"20180708144342.11922-7-dev+git@drbeat.li","threadId":"48839","inReplyTo":"20180708144342.11922-1-dev+git@drbeat.li","subject":"[RFC PATCH 6/6] utf8.c: avoid char overflow","fromName":"Beat Bolli","fromEmail":"dev+git@drbeat.li","sentAt":"2018-07-08T14:43:42Z","receivedAt":"2018-07-08T14:44:17Z","isPatch":true,"sender":{"key":"dev+git@drbeat.li","avatar":"https://avatars.githubusercontent.com/u/21444?v=4"},"body":"In ISO C, char constants must be in the range -128..127. Change the BOM\nconstants to unsigned char to avoid overflow.\n\nSigned-off-by: Beat Bolli <dev+git@drbeat.li>\n---\n utf8.c | 10 +++++-----\n 1 file changed, 5 insertions(+), 5 deletions(-)\n\ndiff --git a/utf8.c b/utf8.c\nindex d55e20c641..833ce00617 100644\n--- a/utf8.c\n+++ b/utf8.c\n@@ -561,15 +561,15 @@ char *reencode_string_len(const char *in, int insz,\n #endif\n \n static int has_bom_prefix(const char *data, size_t len,\n-\t\t\t  const char *bom, size_t bom_len)\n+\t\t\t  const unsigned char *bom, size_t bom_len)\n {\n \treturn data && bom && (len >= bom_len) && !memcmp(data, bom, bom_len);\n }\n \n-static const char utf16_be_bom[] = {0xFE, 0xFF};\n-static const char utf16_le_bom[] = {0xFF, 0xFE};\n-static const char utf32_be_bom[] = {0x00, 0x00, 0xFE, 0xFF};\n-static const char utf32_le_bom[] = {0xFF, 0xFE, 0x00, 0x00};\n+static const unsigned char utf16_be_bom[] = {0xFE, 0xFF};\n+static const unsigned char utf16_le_bom[] = {0xFF, 0xFE};\n+static const unsigned char utf32_be_bom[] = {0x00, 0x00, 0xFE, 0xFF};\n+static const unsigned char utf32_le_bom[] = {0xFF, 0xFE, 0x00, 0x00};\n \n int has_prohibited_utf_bom(const char *enc, const char *data, size_t len)\n {\n-- \n2.15.0.rc1.299.gda03b47c3\n\n"},{"id":"351872","messageId":"20180708144342.11922-2-dev+git@drbeat.li","threadId":"48839","inReplyTo":"20180708144342.11922-1-dev+git@drbeat.li","subject":"[RFC PATCH 1/6] connect.h: avoid forward declaration of an enum","fromName":"Beat Bolli","fromEmail":"dev+git@drbeat.li","sentAt":"2018-07-08T14:43:37Z","receivedAt":"2018-07-08T14:44:21Z","isPatch":true,"sender":{"key":"dev+git@drbeat.li","avatar":"https://avatars.githubusercontent.com/u/21444?v=4"},"body":"Include protocol.h to define enum protocol_version.\n\nSigned-off-by: Beat Bolli <dev+git@drbeat.li>\n---\n connect.h | 2 ++\n 1 file changed, 2 insertions(+)\n\ndiff --git a/connect.h b/connect.h\nindex 0e69c6709c..c86f862f2f 100644\n--- a/connect.h\n+++ b/connect.h\n@@ -1,6 +1,8 @@\n #ifndef CONNECT_H\n #define CONNECT_H\n \n+#include \"protocol.h\"   /* for enum protocol_version */\n+\n #define CONNECT_VERBOSE       (1u << 0)\n #define CONNECT_DIAG_URL      (1u << 1)\n #define CONNECT_IPV4          (1u << 2)\n-- \n2.15.0.rc1.299.gda03b47c3\n\n"},{"id":"351873","messageId":"20180708144342.11922-6-dev+git@drbeat.li","threadId":"48839","inReplyTo":"20180708144342.11922-1-dev+git@drbeat.li","subject":"[RFC PATCH 5/6] string-list.c: avoid conversion from void * to function pointer","fromName":"Beat Bolli","fromEmail":"dev+git@drbeat.li","sentAt":"2018-07-08T14:43:41Z","receivedAt":"2018-07-08T14:44:23Z","isPatch":true,"sender":{"key":"dev+git@drbeat.li","avatar":"https://avatars.githubusercontent.com/u/21444?v=4"},"body":"ISO C forbids the conversion of void pointers to function pointers.\nIntroduce a context struct that encapsulates the function pointer.\n\nSigned-off-by: Beat Bolli <dev+git@drbeat.li>\n---\n string-list.c | 18 ++++++++++++++----\n 1 file changed, 14 insertions(+), 4 deletions(-)\n\ndiff --git a/string-list.c b/string-list.c\nindex a0cf0cfe88..771c455098 100644\n--- a/string-list.c\n+++ b/string-list.c\n@@ -224,18 +224,28 @@ struct string_list_item *string_list_append(struct string_list *list,\n \t\t\tlist->strdup_strings ? xstrdup(string) : (char *)string);\n }\n \n+/*\n+ * Encapsulate the compare function pointer because ISO C99 forbids\n+ * casting from void * to a function pointer and vice versa.\n+ */\n+struct string_list_sort_ctx\n+{\n+\tcompare_strings_fn cmp;\n+};\n+\n static int cmp_items(const void *a, const void *b, void *ctx)\n {\n-\tcompare_strings_fn cmp = ctx;\n+\tstruct string_list_sort_ctx *sort_ctx = ctx;\n \tconst struct string_list_item *one = a;\n \tconst struct string_list_item *two = b;\n-\treturn cmp(one->string, two->string);\n+\treturn sort_ctx->cmp(one->string, two->string);\n }\n \n void string_list_sort(struct string_list *list)\n {\n-\tQSORT_S(list->items, list->nr, cmp_items,\n-\t\tlist->cmp ? list->cmp : strcmp);\n+\tstruct string_list_sort_ctx sort_ctx = {list->cmp ? list->cmp : strcmp};\n+\n+\tQSORT_S(list->items, list->nr, cmp_items, &sort_ctx);\n }\n \n struct string_list_item *unsorted_string_list_lookup(struct string_list *list,\n-- \n2.15.0.rc1.299.gda03b47c3\n\n"},{"id":"351888","messageId":"CAPig+cSGj6Pt4OmRAQauZCiVG3PnjWeXm20RtYGbsjdZ5Tk4gg@mail.gmail.com","threadId":"48839","inReplyTo":"20180708144342.11922-5-dev+git@drbeat.li","subject":"Re: [RFC PATCH 4/6] sequencer.c: avoid empty statements at top level","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-07-08T20:54:04Z","receivedAt":"2018-07-08T20:54:19Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sun, Jul 8, 2018 at 10:44 AM Beat Bolli <dev+git@drbeat.li> wrote:\n> The marco GIT_PATH_FUNC expands to a complete statement including the\n> semicolon. Remove two extra trailing semicolons.\n>\n> Signed-off-by: Beat Bolli <dev+git@drbeat.li>\n> ---\n>  sequencer.c | 4 ++--\n>  1 file changed, 2 insertions(+), 2 deletions(-)\n\nWhile you're at it, perhaps it would be a good idea to fix the example\nin path.h which teaches the \"wrong\" way:\n\n/*\n * You can define a static memoized git path like:\n *\n *    static GIT_PATH_FUNC(git_path_foo, \"FOO\");\n *\n * or use one of the global ones below.\n */\n"},{"id":"351891","messageId":"AED9D71037D249F8A56FDF0B1AA48603@PhilipOakley","threadId":"48839","inReplyTo":"CAPig+cSGj6Pt4OmRAQauZCiVG3PnjWeXm20RtYGbsjdZ5Tk4gg@mail.gmail.com","subject":"Re: [RFC PATCH 4/6] sequencer.c: avoid empty statements at top level","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.org","sentAt":"2018-07-08T21:17:55Z","receivedAt":"2018-07-08T21:17:59Z","isPatch":true,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"From: \"Eric Sunshine\" <sunshine@sunshineco.com>\nTo: \"Beat Bolli\" <dev+git@drbeat.li>\n> On Sun, Jul 8, 2018 at 10:44 AM Beat Bolli <dev+git@drbeat.li> wrote:\n>> The marco GIT_PATH_FUNC expands to a complete statement including the\n\ns/marco/macro/\n\n>> semicolon. Remove two extra trailing semicolons.\n>>\n>> Signed-off-by: Beat Bolli <dev+git@drbeat.li>\n>> ---\n>>  sequencer.c | 4 ++--\n>>  1 file changed, 2 insertions(+), 2 deletions(-)\n> \n> While you're at it, perhaps it would be a good idea to fix the example\n> in path.h which teaches the \"wrong\" way:\n> \n> /*\n> * You can define a static memoized git path like:\n> *\n> *    static GIT_PATH_FUNC(git_path_foo, \"FOO\");\n> *\n> * or use one of the global ones below.\n> */\n>\n"},{"id":"351938","messageId":"b1427480e2b70c2ad1ab5873409e5fdc@drbeat.li","threadId":"48839","inReplyTo":"AED9D71037D249F8A56FDF0B1AA48603@PhilipOakley","subject":"Re: [RFC PATCH 4/6] sequencer.c: avoid empty statements at top level","fromName":"","fromEmail":"ig@drbeat.li","sentAt":"2018-07-09T09:37:21Z","receivedAt":"2018-07-09T09:43:19Z","isPatch":true,"sender":{"key":"ig@drbeat.li","avatar":"https://gravatar.com/avatar/e0cf621324d5568114197741c71c2728594b76b994c01f82f1245c8e3c18a015?d=mp&s=160"},"body":"Am 08.07.2018 23:17, schrieb Philip Oakley:\n> From: \"Eric Sunshine\" <sunshine@sunshineco.com>\n> To: \"Beat Bolli\" <dev+git@drbeat.li>\n>> On Sun, Jul 8, 2018 at 10:44 AM Beat Bolli <dev+git@drbeat.li> wrote:\n>>> The marco GIT_PATH_FUNC expands to a complete statement including the\n> \n> s/marco/macro/\n\nACK. In addition, the whole sentence is wrong: GIT_PATH_FUNC defines a\nfunction, not a statement. Will be fixes in a reroll.\n\n>>> semicolon. Remove two extra trailing semicolons.\n>>> \n>>> Signed-off-by: Beat Bolli <dev+git@drbeat.li>\n>>> ---\n>>>  sequencer.c | 4 ++--\n>>>  1 file changed, 2 insertions(+), 2 deletions(-)\n>> \n>> While you're at it, perhaps it would be a good idea to fix the example\n>> in path.h which teaches the \"wrong\" way:\n>> \n>> /*\n>> * You can define a static memoized git path like:\n>> *\n>> *    static GIT_PATH_FUNC(git_path_foo, \"FOO\");\n>> *\n>> * or use one of the global ones below.\n>> */\n\nThanks,\nBeat\n"},{"id":"351941","messageId":"nycvar.QRO.7.76.6.1807091513130.75@tvgsbejvaqbjf.bet","threadId":"48839","inReplyTo":"20180708144342.11922-7-dev+git@drbeat.li","subject":"Re: [RFC PATCH 6/6] utf8.c: avoid char overflow","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-07-09T13:14:43Z","receivedAt":"2018-07-09T13:14:54Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Beat,\n\nOn Sun, 8 Jul 2018, Beat Bolli wrote:\n\n> In ISO C, char constants must be in the range -128..127. Change the BOM\n> constants to unsigned char to avoid overflow.\n> \n> Signed-off-by: Beat Bolli <dev+git@drbeat.li>\n> ---\n>  utf8.c | 10 +++++-----\n>  1 file changed, 5 insertions(+), 5 deletions(-)\n> \n> diff --git a/utf8.c b/utf8.c\n> index d55e20c641..833ce00617 100644\n> --- a/utf8.c\n> +++ b/utf8.c\n> @@ -561,15 +561,15 @@ char *reencode_string_len(const char *in, int insz,\n>  #endif\n>  \n>  static int has_bom_prefix(const char *data, size_t len,\n> -\t\t\t  const char *bom, size_t bom_len)\n> +\t\t\t  const unsigned char *bom, size_t bom_len)\n>  {\n>  \treturn data && bom && (len >= bom_len) && !memcmp(data, bom, bom_len);\n>  }\n>  \n> -static const char utf16_be_bom[] = {0xFE, 0xFF};\n> -static const char utf16_le_bom[] = {0xFF, 0xFE};\n> -static const char utf32_be_bom[] = {0x00, 0x00, 0xFE, 0xFF};\n> -static const char utf32_le_bom[] = {0xFF, 0xFE, 0x00, 0x00};\n> +static const unsigned char utf16_be_bom[] = {0xFE, 0xFF};\n> +static const unsigned char utf16_le_bom[] = {0xFF, 0xFE};\n> +static const unsigned char utf32_be_bom[] = {0x00, 0x00, 0xFE, 0xFF};\n> +static const unsigned char utf32_le_bom[] = {0xFF, 0xFE, 0x00, 0x00};\n\nAn alternative approach that might be easier to read (and avoids the\nconfusion arising from our use of (signed) chars for strings pretty much\neverywhere):\n\n#define FE ((char)0xfe)\n#define FF ((char)0xff)\n\n...\n\nCiao,\nDscho\n"},{"id":"351942","messageId":"nycvar.QRO.7.76.6.1807091539540.75@tvgsbejvaqbjf.bet","threadId":"48839","inReplyTo":"20180708144342.11922-1-dev+git@drbeat.li","subject":"Re: [RFC PATCH 0/6] Compile cleanly in pedantic mode","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-07-09T13:40:28Z","receivedAt":"2018-07-09T13:40:41Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Beat,\n\nOn Sun, 8 Jul 2018, Beat Bolli wrote:\n\n> While developing 6aaded550 (\"builtin/config: work around an unsized\n> array forward declaration\", 2018-07-05), I have compiled Git with\n> CFLAGS=\"-std=c99 -pedantic\".\n> \n> This is an RFC patch series that fixes a few compiler warnings when\n> compiling with these options, always assuming that this is a worthwile\n> goal.\n\nThank you, I think it is safe to remove the RFC label: this is really good\nto have, in order to make it as portable C as possible.\n\nCiao,\nDscho\n"},{"id":"351947","messageId":"0ceeb342fec1d0868b81cd64941df53c@drbeat.li","threadId":"48839","inReplyTo":"nycvar.QRO.7.76.6.1807091513130.75@tvgsbejvaqbjf.bet","subject":"Re: [RFC PATCH 6/6] utf8.c: avoid char overflow","fromName":"Beat Bolli","fromEmail":"dev+git@drbeat.li","sentAt":"2018-07-09T14:48:28Z","receivedAt":"2018-07-09T15:00:25Z","isPatch":true,"sender":{"key":"dev+git@drbeat.li","avatar":"https://avatars.githubusercontent.com/u/21444?v=4"},"body":"Hi Dscho\n\nAm 09.07.2018 15:14, schrieb Johannes Schindelin:\n> Hi Beat,\n> \n> On Sun, 8 Jul 2018, Beat Bolli wrote:\n> \n>> In ISO C, char constants must be in the range -128..127. Change the \n>> BOM\n>> constants to unsigned char to avoid overflow.\n>> \n>> Signed-off-by: Beat Bolli <dev+git@drbeat.li>\n>> ---\n>>  utf8.c | 10 +++++-----\n>>  1 file changed, 5 insertions(+), 5 deletions(-)\n>> \n>> diff --git a/utf8.c b/utf8.c\n>> index d55e20c641..833ce00617 100644\n>> --- a/utf8.c\n>> +++ b/utf8.c\n>> @@ -561,15 +561,15 @@ char *reencode_string_len(const char *in, int \n>> insz,\n>>  #endif\n>> \n>>  static int has_bom_prefix(const char *data, size_t len,\n>> -\t\t\t  const char *bom, size_t bom_len)\n>> +\t\t\t  const unsigned char *bom, size_t bom_len)\n>>  {\n>>  \treturn data && bom && (len >= bom_len) && !memcmp(data, bom, \n>> bom_len);\n>>  }\n>> \n>> -static const char utf16_be_bom[] = {0xFE, 0xFF};\n>> -static const char utf16_le_bom[] = {0xFF, 0xFE};\n>> -static const char utf32_be_bom[] = {0x00, 0x00, 0xFE, 0xFF};\n>> -static const char utf32_le_bom[] = {0xFF, 0xFE, 0x00, 0x00};\n>> +static const unsigned char utf16_be_bom[] = {0xFE, 0xFF};\n>> +static const unsigned char utf16_le_bom[] = {0xFF, 0xFE};\n>> +static const unsigned char utf32_be_bom[] = {0x00, 0x00, 0xFE, 0xFF};\n>> +static const unsigned char utf32_le_bom[] = {0xFF, 0xFE, 0x00, 0x00};\n> \n> An alternative approach that might be easier to read (and avoids the\n> confusion arising from our use of (signed) chars for strings pretty \n> much\n> everywhere):\n> \n> #define FE ((char)0xfe)\n> #define FF ((char)0xff)\n> \n> ...\n\nI have tried this first (without the macros, though), and thought it \nlooked\nreally ugly. That's why I chose this solution. The usage is pretty local \nand\nclose to function has_bom_prefix().\n\nWould an explaining comment help?\n\nBeat\n"},{"id":"351948","messageId":"e3df2644b59b170e26b2a7c0d3978331@drbeat.li","threadId":"48839","inReplyTo":"0ceeb342fec1d0868b81cd64941df53c@drbeat.li","subject":"Re: [RFC PATCH 6/6] utf8.c: avoid char overflow","fromName":"Beat Bolli","fromEmail":"dev+git@drbeat.li","sentAt":"2018-07-09T15:45:05Z","receivedAt":"2018-07-09T15:45:12Z","isPatch":true,"sender":{"key":"dev+git@drbeat.li","avatar":"https://avatars.githubusercontent.com/u/21444?v=4"},"body":"Am 09.07.2018 16:48, schrieb Beat Bolli:\n> Hi Dscho\n> \n> Am 09.07.2018 15:14, schrieb Johannes Schindelin:\n>> Hi Beat,\n>> \n>> On Sun, 8 Jul 2018, Beat Bolli wrote:\n>> \n>>> In ISO C, char constants must be in the range -128..127. Change the \n>>> BOM\n>>> constants to unsigned char to avoid overflow.\n>>> \n>>> Signed-off-by: Beat Bolli <dev+git@drbeat.li>\n>>> ---\n>>>  utf8.c | 10 +++++-----\n>>>  1 file changed, 5 insertions(+), 5 deletions(-)\n>>> \n>>> diff --git a/utf8.c b/utf8.c\n>>> index d55e20c641..833ce00617 100644\n>>> --- a/utf8.c\n>>> +++ b/utf8.c\n>>> @@ -561,15 +561,15 @@ char *reencode_string_len(const char *in, int \n>>> insz,\n>>>  #endif\n>>> \n>>>  static int has_bom_prefix(const char *data, size_t len,\n>>> -\t\t\t  const char *bom, size_t bom_len)\n>>> +\t\t\t  const unsigned char *bom, size_t bom_len)\n>>>  {\n>>>  \treturn data && bom && (len >= bom_len) && !memcmp(data, bom, \n>>> bom_len);\n>>>  }\n>>> \n>>> -static const char utf16_be_bom[] = {0xFE, 0xFF};\n>>> -static const char utf16_le_bom[] = {0xFF, 0xFE};\n>>> -static const char utf32_be_bom[] = {0x00, 0x00, 0xFE, 0xFF};\n>>> -static const char utf32_le_bom[] = {0xFF, 0xFE, 0x00, 0x00};\n>>> +static const unsigned char utf16_be_bom[] = {0xFE, 0xFF};\n>>> +static const unsigned char utf16_le_bom[] = {0xFF, 0xFE};\n>>> +static const unsigned char utf32_be_bom[] = {0x00, 0x00, 0xFE, \n>>> 0xFF};\n>>> +static const unsigned char utf32_le_bom[] = {0xFF, 0xFE, 0x00, \n>>> 0x00};\n>> \n>> An alternative approach that might be easier to read (and avoids the\n>> confusion arising from our use of (signed) chars for strings pretty \n>> much\n>> everywhere):\n>> \n>> #define FE ((char)0xfe)\n>> #define FF ((char)0xff)\n>> \n>> ...\n> \n> I have tried this first (without the macros, though), and thought it \n> looked\n> really ugly. That's why I chose this solution. The usage is pretty \n> local and\n> close to function has_bom_prefix().\n> \n> Would an explaining comment help?\n\nI have found an even simpler solution. Use proper char literals.\n\nI will put this into v2.\n\nRegards,\nBeat\n\n\ndiff --git a/utf8.c b/utf8.c\nindex d55e20c641..982217eec9 100644\n--- a/utf8.c\n+++ b/utf8.c\n@@ -566,10 +566,10 @@ static int has_bom_prefix(const char *data, size_t \nlen,\n         return data && bom && (len >= bom_len) && !memcmp(data, bom, \nbom_len);\n  }\n\n-static const char utf16_be_bom[] = {0xFE, 0xFF};\n-static const char utf16_le_bom[] = {0xFF, 0xFE};\n-static const char utf32_be_bom[] = {0x00, 0x00, 0xFE, 0xFF};\n-static const char utf32_le_bom[] = {0xFF, 0xFE, 0x00, 0x00};\n+static const char utf16_be_bom[] = {'\\xFE', '\\xFF'};\n+static const char utf16_le_bom[] = {'\\xFF', '\\xFE'};\n+static const char utf32_be_bom[] = {'\\0', '\\0', '\\xFE', '\\xFF'};\n+static const char utf32_le_bom[] = {'\\xFF', '\\xFE', '\\0', '\\0'};\n\n  int has_prohibited_utf_bom(const char *enc, const char *data, size_t \nlen)\n  {\n"},{"id":"351953","messageId":"xmqqh8l8fka2.fsf@gitster-ct.c.googlers.com","threadId":"48839","inReplyTo":"20180708144342.11922-1-dev+git@drbeat.li","subject":"Re: [RFC PATCH 0/6] Compile cleanly in pedantic mode","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-07-09T16:25:57Z","receivedAt":"2018-07-09T16:26:02Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Beat Bolli <dev+git@drbeat.li> writes:\n\n> While developing 6aaded550 (\"builtin/config: work around an unsized\n> array forward declaration\", 2018-07-05), I have compiled Git with\n> CFLAGS=\"-std=c99 -pedantic\".\n\nNice.  I also pretty recently realized that I stopped building with\nthe pedantic option by accident, probably when DEVELOPER build knob\nwas added to the main Makefile, and have been disturbed by these\nfailures.\n"},{"id":"351954","messageId":"xmqqd0vwfjxt.fsf@gitster-ct.c.googlers.com","threadId":"48839","inReplyTo":"0ceeb342fec1d0868b81cd64941df53c@drbeat.li","subject":"Re: [RFC PATCH 6/6] utf8.c: avoid char overflow","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-07-09T16:33:18Z","receivedAt":"2018-07-09T16:33:23Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Beat Bolli <dev+git@drbeat.li> writes:\n\n>>> -static const char utf16_be_bom[] = {0xFE, 0xFF};\n>>> -static const char utf16_le_bom[] = {0xFF, 0xFE};\n>>> -static const char utf32_be_bom[] = {0x00, 0x00, 0xFE, 0xFF};\n>>> -static const char utf32_le_bom[] = {0xFF, 0xFE, 0x00, 0x00};\n>>> +static const unsigned char utf16_be_bom[] = {0xFE, 0xFF};\n>>> +static const unsigned char utf16_le_bom[] = {0xFF, 0xFE};\n>>> +static const unsigned char utf32_be_bom[] = {0x00, 0x00, 0xFE, 0xFF};\n>>> +static const unsigned char utf32_le_bom[] = {0xFF, 0xFE, 0x00, 0x00};\n>>\n>> An alternative approach that might be easier to read (and avoids the\n>> confusion arising from our use of (signed) chars for strings pretty\n>> much\n>> everywhere):\n>>\n>> #define FE ((char)0xfe)\n>> #define FF ((char)0xff)\n>>\n>> ...\n>\n> I have tried this first (without the macros, though), and thought\n> it looked really ugly. That's why I chose this solution. The usage\n> is pretty local and close to function has_bom_prefix().\n\nI found that what you posted was already OK, as has_bom_prefix()\nappears only locally in this file and that is the only thing that\ncares about these foo_bom[] constants.  Casting the elements in\nthese arrays to (char) type is also fine and not all that ugly,\nI think, and between the two (but without the macro) I have no\nstrong preference.  I wonder if writing them as '\\376' and '\\377'\nas old timers would helps the compiler, though.\n\n"},{"id":"351969","messageId":"c42bd49e-15b0-2fc6-b92e-3d0108843aea@drbeat.li","threadId":"48839","inReplyTo":"xmqqd0vwfjxt.fsf@gitster-ct.c.googlers.com","subject":"Re: [RFC PATCH 6/6] utf8.c: avoid char overflow","fromName":"Beat Bolli","fromEmail":"dev+git@drbeat.li","sentAt":"2018-07-09T17:56:38Z","receivedAt":"2018-07-09T17:56:49Z","isPatch":true,"sender":{"key":"dev+git@drbeat.li","avatar":"https://avatars.githubusercontent.com/u/21444?v=4"},"body":"On 09.07.18 18:33, Junio C Hamano wrote:\n> Beat Bolli <dev+git@drbeat.li> writes:\n> \n>>>> -static const char utf16_be_bom[] = {0xFE, 0xFF};\n>>>> -static const char utf16_le_bom[] = {0xFF, 0xFE};\n>>>> -static const char utf32_be_bom[] = {0x00, 0x00, 0xFE, 0xFF};\n>>>> -static const char utf32_le_bom[] = {0xFF, 0xFE, 0x00, 0x00};\n>>>> +static const unsigned char utf16_be_bom[] = {0xFE, 0xFF};\n>>>> +static const unsigned char utf16_le_bom[] = {0xFF, 0xFE};\n>>>> +static const unsigned char utf32_be_bom[] = {0x00, 0x00, 0xFE, 0xFF};\n>>>> +static const unsigned char utf32_le_bom[] = {0xFF, 0xFE, 0x00, 0x00};\n>>>\n>>> An alternative approach that might be easier to read (and avoids the\n>>> confusion arising from our use of (signed) chars for strings pretty\n>>> much\n>>> everywhere):\n>>>\n>>> #define FE ((char)0xfe)\n>>> #define FF ((char)0xff)\n>>>\n>>> ...\n>>\n>> I have tried this first (without the macros, though), and thought\n>> it looked really ugly. That's why I chose this solution. The usage\n>> is pretty local and close to function has_bom_prefix().\n> \n> I found that what you posted was already OK, as has_bom_prefix()\n> appears only locally in this file and that is the only thing that\n> cares about these foo_bom[] constants.  Casting the elements in\n> these arrays to (char) type is also fine and not all that ugly,\n> I think, and between the two (but without the macro) I have no\n> strong preference.  I wonder if writing them as '\\376' and '\\377'\n> as old timers would helps the compiler, though.\n> \n\nYes, it does, as I found out in\nhttps://public-inbox.org/git/e3df2644b59b170e26b2a7c0d3978331@drbeat.li/\n\nBut I prefer hex; it's closer to the usual definition of the BOM bytes.\n\nBeat\n"},{"id":"351973","messageId":"xmqqd0vwe0ij.fsf@gitster-ct.c.googlers.com","threadId":"48839","inReplyTo":"xmqqd0vwfjxt.fsf@gitster-ct.c.googlers.com","subject":"Re: [RFC PATCH 6/6] utf8.c: avoid char overflow","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-07-09T18:18:12Z","receivedAt":"2018-07-09T18:18:17Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Beat Bolli <dev+git@drbeat.li> writes:\n>\n>>>> -static const char utf16_be_bom[] = {0xFE, 0xFF};\n>>>> -static const char utf16_le_bom[] = {0xFF, 0xFE};\n>>>> -static const char utf32_be_bom[] = {0x00, 0x00, 0xFE, 0xFF};\n>>>> -static const char utf32_le_bom[] = {0xFF, 0xFE, 0x00, 0x00};\n>>>> +static const unsigned char utf16_be_bom[] = {0xFE, 0xFF};\n>>>> +static const unsigned char utf16_le_bom[] = {0xFF, 0xFE};\n>>>> +static const unsigned char utf32_be_bom[] = {0x00, 0x00, 0xFE, 0xFF};\n>>>> +static const unsigned char utf32_le_bom[] = {0xFF, 0xFE, 0x00, 0x00};\n>>>\n>>> An alternative approach that might be easier to read (and avoids the\n>>> confusion arising from our use of (signed) chars for strings pretty\n>>> much\n>>> everywhere):\n>>>\n>>> #define FE ((char)0xfe)\n>>> #define FF ((char)0xff)\n>>>\n>>> ...\n>>\n>> I have tried this first (without the macros, though), and thought\n>> it looked really ugly. That's why I chose this solution. The usage\n>> is pretty local and close to function has_bom_prefix().\n>\n> I found that what you posted was already OK, as has_bom_prefix()\n> appears only locally in this file and that is the only thing that\n> cares about these foo_bom[] constants.  Casting the elements in\n> these arrays to (char) type is also fine and not all that ugly,\n> I think, and between the two (but without the macro) I have no\n> strong preference.  I wonder if writing them as '\\376' and '\\377'\n> as old timers would helps the compiler, though.\n\nHeh, I should have read the remainder of my mailbox to learn that\nyou came to the same conclusion in response to Dscho's comment\nbefore saying the above.  Hex is OK as all our compilers would be\ncapable of gloking '\\xff' (there is a rather unusual '\\x20' already\nin builtin/clone.c).\n\nThanks.\n\n\n"},{"id":"351978","messageId":"20180709184632.GA9601@sigill.intra.peff.net","threadId":"48839","inReplyTo":"20180708144342.11922-3-dev+git@drbeat.li","subject":"Re: [RFC PATCH 2/6] refs/refs-internal.h: avoid forward declaration of an enum","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-07-09T18:46:32Z","receivedAt":"2018-07-09T18:46:37Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Jul 08, 2018 at 04:43:38PM +0200, Beat Bolli wrote:\n\n> diff --git a/refs/refs-internal.h b/refs/refs-internal.h\n> index dd834314bd..a78b5cb803 100644\n> --- a/refs/refs-internal.h\n> +++ b/refs/refs-internal.h\n> @@ -1,6 +1,8 @@\n>  #ifndef REFS_REFS_INTERNAL_H\n>  #define REFS_REFS_INTERNAL_H\n>  \n> +#include \"iterator.h\"   /* for enum iterator_selection */\n\nIMHO this kind of comment does more harm than good, because it is so\nprone to going stale (nobody is going to bother updating it when they\nadd new dependencies on iterator.h). Anybody who is interested in the\noriginal reason can use \"git blame\" to dig up your commit message. And\nanybody who is thinking about deleting that line would need to dig into\nwhether anything had been added in the meantime that also requires the\ninclude.\n\nSo at best it's redundant, and at worst it's slightly misleading. :)\n\nNot worth a re-roll by itself, but it looked like you had a few other\nbits in the other patches to address.\n\nOther than this minor quibble, the whole series looks good to me, modulo\nthe existing review.\n\n-Peff\n"},{"id":"351984","messageId":"20180709192537.18564-1-dev+git@drbeat.li","threadId":"48839","inReplyTo":"20180708144342.11922-1-dev+git@drbeat.li","subject":"[PATCH 0/6] Compile cleanly in pedantic mode","fromName":"Beat Bolli","fromEmail":"dev+git@drbeat.li","sentAt":"2018-07-09T19:25:31Z","receivedAt":"2018-07-09T19:25:53Z","isPatch":true,"sender":{"key":"dev+git@drbeat.li","avatar":"https://avatars.githubusercontent.com/u/21444?v=4"},"body":"While developing 6aaded550 (\"builtin/config: work around an unsized\narray forward declaration\", 2018-07-05), I have compiled Git with\nCFLAGS=\"-std=c99 -pedantic\".\n\nThis series fixes a few compiler warnings when compiling with these\noptions.\n\nNote that all warnings were produced by -pedantic; the C99 standard\noption by itself didn't cause any of them.\n\nThe warnings were:\n\n1) Char arrays initialized from a parenthesized string.\n\n        Suppressed by defining USE_PARENS_AROUND_GETTEXT_N to 0\n        globally. This was done just to keep the amount of warnings\n        manageable; this series leaves that knob alone. The advantage of\n        not mistakenly concatenating two translated strings is greater.\n\n2) connect.h, refs/refs-internal.h: Forward reference to an enum.\n\n        Added two #includes that define the enums. This was already\n        (inconclusively) talked about in [0].\n\n3) convert.c: Invalid escape sequence \"\\e\".\n\n        Replaced with \"\\033\".\n\n4) sequencer.c: Empty statements at top level.\n\n        Removed the extra semicolons.\n\n5) string-list.c: Forbidden to cast from void * to a function pointer and\n   vice versa.\n\n        Encapsulated the function pointer in a context struct. This is\n        controversial because it has a performance impact, namely one\n        additional pointer dereference per string comparison. An\n        alternative might be to use multiple casts via intptr_t. But\n        I'm not sure if this is worth the trouble.\n\n6) utf8.c: overflow of char values.\n\n        Used proper char literals for the BOM constants.\n\nThis series has patches for 2) to 6).\n\nRegards,\nBeat\n\n[0] https://public-inbox.org/git/53ab8626-f862-a732-b369-abeab69a468f@ramsayjones.plus.com/T/\n\n\nBeat Bolli (6):\n  connect.h: avoid forward declaration of an enum\n  refs/refs-internal.h: avoid forward declaration of an enum\n  convert.c: replace \"\\e\" escapes with \"\\033\".\n  sequencer.c: avoid empty statements at top level\n  string-list.c: avoid conversion from void * to function pointer\n  utf8.c: avoid char overflow\n\n connect.h            |  2 ++\n convert.c            |  2 +-\n path.h               |  2 +-\n refs/refs-internal.h |  2 ++\n sequencer.c          |  4 ++--\n string-list.c        | 18 ++++++++++++++----\n utf8.c               |  8 ++++----\n 7 files changed, 26 insertions(+), 12 deletions(-)\n\n\nInterdiff from the RFC series:\n\ndiff --git a/path.h b/path.h\nindex 1ccd0373c9..fc9d3487a0 100644\n--- a/path.h\n+++ b/path.h\n@@ -147,7 +147,7 @@ extern void report_linked_checkout_garbage(void);\n /*\n  * You can define a static memoized git path like:\n  *\n- *    static GIT_PATH_FUNC(git_path_foo, \"FOO\");\n+ *    static GIT_PATH_FUNC(git_path_foo, \"FOO\")\n  *\n  * or use one of the global ones below.\n  */\ndiff --git a/utf8.c b/utf8.c\nindex 833ce00617..982217eec9 100644\n--- a/utf8.c\n+++ b/utf8.c\n@@ -561,15 +561,15 @@ char *reencode_string_len(const char *in, int insz,\n #endif\n\n static int has_bom_prefix(const char *data, size_t len,\n-                         const unsigned char *bom, size_t bom_len)\n+                         const char *bom, size_t bom_len)\n {\n        return data && bom && (len >= bom_len) && !memcmp(data, bom, bom_len);\n }\n\n-static const unsigned char utf16_be_bom[] = {0xFE, 0xFF};\n-static const unsigned char utf16_le_bom[] = {0xFF, 0xFE};\n-static const unsigned char utf32_be_bom[] = {0x00, 0x00, 0xFE, 0xFF};\n-static const unsigned char utf32_le_bom[] = {0xFF, 0xFE, 0x00, 0x00};\n+static const char utf16_be_bom[] = {'\\xFE', '\\xFF'};\n+static const char utf16_le_bom[] = {'\\xFF', '\\xFE'};\n+static const char utf32_be_bom[] = {'\\0', '\\0', '\\xFE', '\\xFF'};\n+static const char utf32_le_bom[] = {'\\xFF', '\\xFE', '\\0', '\\0'};\n\n int has_prohibited_utf_bom(const char *enc, const char *data, size_t len)\n {\n\n-- \n2.18.0.203.gfac676dfb9\n"},{"id":"351985","messageId":"20180709192537.18564-2-dev+git@drbeat.li","threadId":"48839","inReplyTo":"20180708144342.11922-1-dev+git@drbeat.li","subject":"[PATCH 1/6] connect.h: avoid forward declaration of an enum","fromName":"Beat Bolli","fromEmail":"dev+git@drbeat.li","sentAt":"2018-07-09T19:25:32Z","receivedAt":"2018-07-09T19:26:06Z","isPatch":true,"sender":{"key":"dev+git@drbeat.li","avatar":"https://avatars.githubusercontent.com/u/21444?v=4"},"body":"Include protocol.h to define enum protocol_version.\n\nSigned-off-by: Beat Bolli <dev+git@drbeat.li>\n---\n connect.h | 2 ++\n 1 file changed, 2 insertions(+)\n\ndiff --git a/connect.h b/connect.h\nindex 0e69c6709c..c86f862f2f 100644\n--- a/connect.h\n+++ b/connect.h\n@@ -1,6 +1,8 @@\n #ifndef CONNECT_H\n #define CONNECT_H\n \n+#include \"protocol.h\"   /* for enum protocol_version */\n+\n #define CONNECT_VERBOSE       (1u << 0)\n #define CONNECT_DIAG_URL      (1u << 1)\n #define CONNECT_IPV4          (1u << 2)\n-- \n2.18.0.203.gfac676dfb9\n\n"},{"id":"351986","messageId":"20180709192537.18564-6-dev+git@drbeat.li","threadId":"48839","inReplyTo":"20180708144342.11922-1-dev+git@drbeat.li","subject":"[PATCH 5/6] string-list.c: avoid conversion from void * to function pointer","fromName":"Beat Bolli","fromEmail":"dev+git@drbeat.li","sentAt":"2018-07-09T19:25:36Z","receivedAt":"2018-07-09T19:26:07Z","isPatch":true,"sender":{"key":"dev+git@drbeat.li","avatar":"https://avatars.githubusercontent.com/u/21444?v=4"},"body":"ISO C forbids the conversion of void pointers to function pointers.\nIntroduce a context struct that encapsulates the function pointer.\n\nSigned-off-by: Beat Bolli <dev+git@drbeat.li>\n---\n string-list.c | 18 ++++++++++++++----\n 1 file changed, 14 insertions(+), 4 deletions(-)\n\ndiff --git a/string-list.c b/string-list.c\nindex a0cf0cfe88..771c455098 100644\n--- a/string-list.c\n+++ b/string-list.c\n@@ -224,18 +224,28 @@ struct string_list_item *string_list_append(struct string_list *list,\n \t\t\tlist->strdup_strings ? xstrdup(string) : (char *)string);\n }\n \n+/*\n+ * Encapsulate the compare function pointer because ISO C99 forbids\n+ * casting from void * to a function pointer and vice versa.\n+ */\n+struct string_list_sort_ctx\n+{\n+\tcompare_strings_fn cmp;\n+};\n+\n static int cmp_items(const void *a, const void *b, void *ctx)\n {\n-\tcompare_strings_fn cmp = ctx;\n+\tstruct string_list_sort_ctx *sort_ctx = ctx;\n \tconst struct string_list_item *one = a;\n \tconst struct string_list_item *two = b;\n-\treturn cmp(one->string, two->string);\n+\treturn sort_ctx->cmp(one->string, two->string);\n }\n \n void string_list_sort(struct string_list *list)\n {\n-\tQSORT_S(list->items, list->nr, cmp_items,\n-\t\tlist->cmp ? list->cmp : strcmp);\n+\tstruct string_list_sort_ctx sort_ctx = {list->cmp ? list->cmp : strcmp};\n+\n+\tQSORT_S(list->items, list->nr, cmp_items, &sort_ctx);\n }\n \n struct string_list_item *unsorted_string_list_lookup(struct string_list *list,\n-- \n2.18.0.203.gfac676dfb9\n\n"},{"id":"351987","messageId":"20180709192537.18564-4-dev+git@drbeat.li","threadId":"48839","inReplyTo":"20180708144342.11922-1-dev+git@drbeat.li","subject":"[PATCH 3/6] convert.c: replace \"\\e\" escapes with \"\\033\".","fromName":"Beat Bolli","fromEmail":"dev+git@drbeat.li","sentAt":"2018-07-09T19:25:34Z","receivedAt":"2018-07-09T19:26:11Z","isPatch":true,"sender":{"key":"dev+git@drbeat.li","avatar":"https://avatars.githubusercontent.com/u/21444?v=4"},"body":"The \"\\e\" escape is not defined in ISO C.\n\nWhile on this line, add a missing space after the comma.\n\nSigned-off-by: Beat Bolli <dev+git@drbeat.li>\n---\n convert.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/convert.c b/convert.c\nindex 64d0d30e08..edebb946f5 100644\n--- a/convert.c\n+++ b/convert.c\n@@ -334,7 +334,7 @@ static void trace_encoding(const char *context, const char *path,\n \tstrbuf_addf(&trace, \"%s (%s, considered %s):\\n\", context, path, encoding);\n \tfor (i = 0; i < len && buf; ++i) {\n \t\tstrbuf_addf(\n-\t\t\t&trace,\"| \\e[2m%2i:\\e[0m %2x \\e[2m%c\\e[0m%c\",\n+\t\t\t&trace, \"| \\033[2m%2i:\\033[0m %2x \\033[2m%c\\033[0m%c\",\n \t\t\ti,\n \t\t\t(unsigned char) buf[i],\n \t\t\t(buf[i] > 32 && buf[i] < 127 ? buf[i] : ' '),\n-- \n2.18.0.203.gfac676dfb9\n\n"},{"id":"351988","messageId":"20180709192537.18564-3-dev+git@drbeat.li","threadId":"48839","inReplyTo":"20180708144342.11922-1-dev+git@drbeat.li","subject":"[PATCH 2/6] refs/refs-internal.h: avoid forward declaration of an enum","fromName":"Beat Bolli","fromEmail":"dev+git@drbeat.li","sentAt":"2018-07-09T19:25:33Z","receivedAt":"2018-07-09T19:26:13Z","isPatch":true,"sender":{"key":"dev+git@drbeat.li","avatar":"https://avatars.githubusercontent.com/u/21444?v=4"},"body":"Include iterator.h to define enum iterator_selection.\n\nSigned-off-by: Beat Bolli <dev+git@drbeat.li>\n---\n refs/refs-internal.h | 2 ++\n 1 file changed, 2 insertions(+)\n\ndiff --git a/refs/refs-internal.h b/refs/refs-internal.h\nindex dd834314bd..a78b5cb803 100644\n--- a/refs/refs-internal.h\n+++ b/refs/refs-internal.h\n@@ -1,6 +1,8 @@\n #ifndef REFS_REFS_INTERNAL_H\n #define REFS_REFS_INTERNAL_H\n \n+#include \"iterator.h\"   /* for enum iterator_selection */\n+\n /*\n  * Data structures and functions for the internal use of the refs\n  * module. Code outside of the refs module should use only the public\n-- \n2.18.0.203.gfac676dfb9\n\n"},{"id":"351989","messageId":"20180709192537.18564-7-dev+git@drbeat.li","threadId":"48839","inReplyTo":"20180708144342.11922-1-dev+git@drbeat.li","subject":"[PATCH 6/6] utf8.c: avoid char overflow","fromName":"Beat Bolli","fromEmail":"dev+git@drbeat.li","sentAt":"2018-07-09T19:25:37Z","receivedAt":"2018-07-09T19:26:14Z","isPatch":true,"sender":{"key":"dev+git@drbeat.li","avatar":"https://avatars.githubusercontent.com/u/21444?v=4"},"body":"In ISO C, char constants must be in the range -128..127. Change the BOM\nconstants to char literals to avoid overflow.\n\nSigned-off-by: Beat Bolli <dev+git@drbeat.li>\n---\n utf8.c | 8 ++++----\n 1 file changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/utf8.c b/utf8.c\nindex d55e20c641..982217eec9 100644\n--- a/utf8.c\n+++ b/utf8.c\n@@ -566,10 +566,10 @@ static int has_bom_prefix(const char *data, size_t len,\n \treturn data && bom && (len >= bom_len) && !memcmp(data, bom, bom_len);\n }\n \n-static const char utf16_be_bom[] = {0xFE, 0xFF};\n-static const char utf16_le_bom[] = {0xFF, 0xFE};\n-static const char utf32_be_bom[] = {0x00, 0x00, 0xFE, 0xFF};\n-static const char utf32_le_bom[] = {0xFF, 0xFE, 0x00, 0x00};\n+static const char utf16_be_bom[] = {'\\xFE', '\\xFF'};\n+static const char utf16_le_bom[] = {'\\xFF', '\\xFE'};\n+static const char utf32_be_bom[] = {'\\0', '\\0', '\\xFE', '\\xFF'};\n+static const char utf32_le_bom[] = {'\\xFF', '\\xFE', '\\0', '\\0'};\n \n int has_prohibited_utf_bom(const char *enc, const char *data, size_t len)\n {\n-- \n2.18.0.203.gfac676dfb9\n\n"},{"id":"351990","messageId":"20180709192537.18564-5-dev+git@drbeat.li","threadId":"48839","inReplyTo":"20180708144342.11922-1-dev+git@drbeat.li","subject":"[PATCH 4/6] sequencer.c: avoid empty statements at top level","fromName":"Beat Bolli","fromEmail":"dev+git@drbeat.li","sentAt":"2018-07-09T19:25:35Z","receivedAt":"2018-07-09T19:26:18Z","isPatch":true,"sender":{"key":"dev+git@drbeat.li","avatar":"https://avatars.githubusercontent.com/u/21444?v=4"},"body":"The macro GIT_PATH_FUNC expands to a function definition that ends with\na closing brace. Remove two extra semicolons.\n\nWhile at it, fix the example in path.h.\n\nSigned-off-by: Beat Bolli <dev+git@drbeat.li>\n---\n path.h      | 2 +-\n sequencer.c | 4 ++--\n 2 files changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/path.h b/path.h\nindex 1ccd0373c9..fc9d3487a0 100644\n--- a/path.h\n+++ b/path.h\n@@ -147,7 +147,7 @@ extern void report_linked_checkout_garbage(void);\n /*\n  * You can define a static memoized git path like:\n  *\n- *    static GIT_PATH_FUNC(git_path_foo, \"FOO\");\n+ *    static GIT_PATH_FUNC(git_path_foo, \"FOO\")\n  *\n  * or use one of the global ones below.\n  */\ndiff --git a/sequencer.c b/sequencer.c\nindex 5354d4d51e..66e7073995 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -62,12 +62,12 @@ static GIT_PATH_FUNC(rebase_path_done, \"rebase-merge/done\")\n  * The file to keep track of how many commands were already processed (e.g.\n  * for the prompt).\n  */\n-static GIT_PATH_FUNC(rebase_path_msgnum, \"rebase-merge/msgnum\");\n+static GIT_PATH_FUNC(rebase_path_msgnum, \"rebase-merge/msgnum\")\n /*\n  * The file to keep track of how many commands are to be processed in total\n  * (e.g. for the prompt).\n  */\n-static GIT_PATH_FUNC(rebase_path_msgtotal, \"rebase-merge/end\");\n+static GIT_PATH_FUNC(rebase_path_msgtotal, \"rebase-merge/end\")\n /*\n  * The commit message that is planned to be used for any changes that\n  * need to be committed following a user interaction.\n-- \n2.18.0.203.gfac676dfb9\n\n"},{"id":"351992","messageId":"a3e6c187-74a5-1858-b207-b52d409b11be@drbeat.li","threadId":"48839","inReplyTo":"20180709184632.GA9601@sigill.intra.peff.net","subject":"Re: [RFC PATCH 2/6] refs/refs-internal.h: avoid forward declaration of an enum","fromName":"Beat Bolli","fromEmail":"dev+git@drbeat.li","sentAt":"2018-07-09T19:30:12Z","receivedAt":"2018-07-09T19:30:18Z","isPatch":true,"sender":{"key":"dev+git@drbeat.li","avatar":"https://avatars.githubusercontent.com/u/21444?v=4"},"body":"On 09.07.18 20:46, Jeff King wrote:\n> On Sun, Jul 08, 2018 at 04:43:38PM +0200, Beat Bolli wrote:\n> \n>> diff --git a/refs/refs-internal.h b/refs/refs-internal.h\n>> index dd834314bd..a78b5cb803 100644\n>> --- a/refs/refs-internal.h\n>> +++ b/refs/refs-internal.h\n>> @@ -1,6 +1,8 @@\n>>  #ifndef REFS_REFS_INTERNAL_H\n>>  #define REFS_REFS_INTERNAL_H\n>>  \n>> +#include \"iterator.h\"   /* for enum iterator_selection */\n> \n> IMHO this kind of comment does more harm than good, because it is so\n> prone to going stale (nobody is going to bother updating it when they\n> add new dependencies on iterator.h). Anybody who is interested in the\n> original reason can use \"git blame\" to dig up your commit message. And\n> anybody who is thinking about deleting that line would need to dig into\n> whether anything had been added in the meantime that also requires the\n> include.\n> \n> So at best it's redundant, and at worst it's slightly misleading. :)\n> \n> Not worth a re-roll by itself, but it looked like you had a few other\n> bits in the other patches to address.\n> \n> Other than this minor quibble, the whole series looks good to me, modulo\n> the existing review.\n> \n> -Peff\n> \n\nOoosp, I've just sent the non-RFC reroll without this change.\n\nJunio, would you squash this into [1/6] and [2/6], please (if you agree,\nof course :-)\n\nBeat\n"},{"id":"352002","messageId":"nycvar.QRO.7.76.6.1807092203110.75@tvgsbejvaqbjf.bet","threadId":"48839","inReplyTo":"0ceeb342fec1d0868b81cd64941df53c@drbeat.li","subject":"Re: [RFC PATCH 6/6] utf8.c: avoid char overflow","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-07-09T20:04:00Z","receivedAt":"2018-07-09T20:04:11Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Beat,\n\nOn Mon, 9 Jul 2018, Beat Bolli wrote:\n\n> Am 09.07.2018 15:14, schrieb Johannes Schindelin:\n> > \n> > On Sun, 8 Jul 2018, Beat Bolli wrote:\n> > \n> > > In ISO C, char constants must be in the range -128..127. Change the BOM\n> > > constants to unsigned char to avoid overflow.\n> > > \n> > > Signed-off-by: Beat Bolli <dev+git@drbeat.li>\n> > > ---\n> > >  utf8.c | 10 +++++-----\n> > >  1 file changed, 5 insertions(+), 5 deletions(-)\n> > > \n> > > diff --git a/utf8.c b/utf8.c\n> > > index d55e20c641..833ce00617 100644\n> > > --- a/utf8.c\n> > > +++ b/utf8.c\n> > > @@ -561,15 +561,15 @@ char *reencode_string_len(const char *in, int insz,\n> > >  #endif\n> > > \n> > > static int has_bom_prefix(const char *data, size_t len,\n> > > -\t\t\t  const char *bom, size_t bom_len)\n> > > +\t\t\t  const unsigned char *bom, size_t bom_len)\n> > >  {\n> > > \treturn data && bom && (len >= bom_len) && !memcmp(data, bom, bom_len);\n> > >  }\n> > > \n> > > -static const char utf16_be_bom[] = {0xFE, 0xFF};\n> > > -static const char utf16_le_bom[] = {0xFF, 0xFE};\n> > > -static const char utf32_be_bom[] = {0x00, 0x00, 0xFE, 0xFF};\n> > > -static const char utf32_le_bom[] = {0xFF, 0xFE, 0x00, 0x00};\n> > > +static const unsigned char utf16_be_bom[] = {0xFE, 0xFF};\n> > > +static const unsigned char utf16_le_bom[] = {0xFF, 0xFE};\n> > > +static const unsigned char utf32_be_bom[] = {0x00, 0x00, 0xFE, 0xFF};\n> > > +static const unsigned char utf32_le_bom[] = {0xFF, 0xFE, 0x00, 0x00};\n> > \n> > An alternative approach that might be easier to read (and avoids the\n> > confusion arising from our use of (signed) chars for strings pretty much\n> > everywhere):\n> > \n> > #define FE ((char)0xfe)\n> > #define FF ((char)0xff)\n> > \n> > ...\n> \n> I have tried this first (without the macros, though), and thought it looked\n> really ugly.\n\nYep, I would totally agree that it would be very ugly without the macros.\n\nWhich is why I suggested the macros instead, in which case it looks\nrelatively elegant to my eyes.\n\nCiao,\nDscho\n"},{"id":"352010","messageId":"402c2b4a-0cd3-05d0-7f19-a218558cbb61@drbeat.li","threadId":"48839","inReplyTo":"20180709192537.18564-1-dev+git@drbeat.li","subject":"Re: [PATCH 0/6] Compile cleanly in pedantic mode","fromName":"Beat Bolli","fromEmail":"dev+git@drbeat.li","sentAt":"2018-07-09T20:25:22Z","receivedAt":"2018-07-09T20:25:28Z","isPatch":true,"sender":{"key":"dev+git@drbeat.li","avatar":"https://avatars.githubusercontent.com/u/21444?v=4"},"body":"On 09.07.18 21:25, Beat Bolli wrote:\n> While developing 6aaded550 (\"builtin/config: work around an unsized\n> array forward declaration\", 2018-07-05), I have compiled Git with\n> CFLAGS=\"-std=c99 -pedantic\".\n> \n> This series fixes a few compiler warnings when compiling with these\n> options.\n\nAs a small aside, I have also compiled all of Git with -std=c11 using\ngcc 8.1. This didn't turn up any new warnings, so we're looking pretty\nfuture-proof in this regard.\n\nCheers,\nBeat\n"},{"id":"352030","messageId":"xmqqo9fgayb6.fsf@gitster-ct.c.googlers.com","threadId":"48839","inReplyTo":"20180708144342.11922-5-dev+git@drbeat.li","subject":"Re: [RFC PATCH 4/6] sequencer.c: avoid empty statements at top level","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-07-09T21:34:05Z","receivedAt":"2018-07-09T21:34:11Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Beat Bolli <dev+git@drbeat.li> writes:\n\n> The marco GIT_PATH_FUNC expands to a complete statement including the\n> semicolon. Remove two extra trailing semicolons.\n\nWait a bit.  The observation in the log message and the\nimplementation of GIT_PATH_FUNC() do not match.\n\n        #define GIT_PATH_FUNC(func, filename) \\\n                const char *func(void) \\\n                { \\\n                        static char *ret; \\\n                        if (!ret) \\\n                                ret = git_pathdup(filename); \\\n                        return ret; \\\n                }\n\nThe code generated does \"include semicolon\" but that is not why the\ncaller should place semicolon after the closing parens.  Perhaps\nreplace \"including the semicolon.\" with something else, like \", and\nadding a semicolon after it not only is unnecessary but is wrong.\"\nor soemthing like that?\n\nIt is a bit unfortunate that we need to live with a slight uglyness\nof the resulting source code, unlike e.g. define_commit_slab() that\ncan (and must) end with a semicolon, which gives us a more natural\nlook.  But that is a separate issue.\n\n>\n> Signed-off-by: Beat Bolli <dev+git@drbeat.li>\n> ---\n>  sequencer.c | 4 ++--\n>  1 file changed, 2 insertions(+), 2 deletions(-)\n>\n> diff --git a/sequencer.c b/sequencer.c\n> index 5354d4d51e..66e7073995 100644\n> --- a/sequencer.c\n> +++ b/sequencer.c\n> @@ -62,12 +62,12 @@ static GIT_PATH_FUNC(rebase_path_done, \"rebase-merge/done\")\n>   * The file to keep track of how many commands were already processed (e.g.\n>   * for the prompt).\n>   */\n> -static GIT_PATH_FUNC(rebase_path_msgnum, \"rebase-merge/msgnum\");\n> +static GIT_PATH_FUNC(rebase_path_msgnum, \"rebase-merge/msgnum\")\n>  /*\n>   * The file to keep track of how many commands are to be processed in total\n>   * (e.g. for the prompt).\n>   */\n> -static GIT_PATH_FUNC(rebase_path_msgtotal, \"rebase-merge/end\");\n> +static GIT_PATH_FUNC(rebase_path_msgtotal, \"rebase-merge/end\")\n>  /*\n>   * The commit message that is planned to be used for any changes that\n>   * need to be committed following a user interaction.\n"},{"id":"352031","messageId":"xmqqk1q4ay6b.fsf@gitster-ct.c.googlers.com","threadId":"48839","inReplyTo":"20180709192537.18564-5-dev+git@drbeat.li","subject":"Re: [PATCH 4/6] sequencer.c: avoid empty statements at top level","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-07-09T21:37:00Z","receivedAt":"2018-07-09T21:37:06Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Beat Bolli <dev+git@drbeat.li> writes:\n\n> The macro GIT_PATH_FUNC expands to a function definition that ends with\n> a closing brace. Remove two extra semicolons.\n\nGood.  Thanks.\n\n>\n> While at it, fix the example in path.h.\n>\n> Signed-off-by: Beat Bolli <dev+git@drbeat.li>\n> ---\n>  path.h      | 2 +-\n>  sequencer.c | 4 ++--\n>  2 files changed, 3 insertions(+), 3 deletions(-)\n>\n> diff --git a/path.h b/path.h\n> index 1ccd0373c9..fc9d3487a0 100644\n> --- a/path.h\n> +++ b/path.h\n> @@ -147,7 +147,7 @@ extern void report_linked_checkout_garbage(void);\n>  /*\n>   * You can define a static memoized git path like:\n>   *\n> - *    static GIT_PATH_FUNC(git_path_foo, \"FOO\");\n> + *    static GIT_PATH_FUNC(git_path_foo, \"FOO\")\n>   *\n>   * or use one of the global ones below.\n>   */\n> diff --git a/sequencer.c b/sequencer.c\n> index 5354d4d51e..66e7073995 100644\n> --- a/sequencer.c\n> +++ b/sequencer.c\n> @@ -62,12 +62,12 @@ static GIT_PATH_FUNC(rebase_path_done, \"rebase-merge/done\")\n>   * The file to keep track of how many commands were already processed (e.g.\n>   * for the prompt).\n>   */\n> -static GIT_PATH_FUNC(rebase_path_msgnum, \"rebase-merge/msgnum\");\n> +static GIT_PATH_FUNC(rebase_path_msgnum, \"rebase-merge/msgnum\")\n>  /*\n>   * The file to keep track of how many commands are to be processed in total\n>   * (e.g. for the prompt).\n>   */\n> -static GIT_PATH_FUNC(rebase_path_msgtotal, \"rebase-merge/end\");\n> +static GIT_PATH_FUNC(rebase_path_msgtotal, \"rebase-merge/end\")\n>  /*\n>   * The commit message that is planned to be used for any changes that\n>   * need to be committed following a user interaction.\n"},{"id":"352032","messageId":"82fa7e95-5e27-bb8a-2fbf-4895bdfea05c@drbeat.li","threadId":"48839","inReplyTo":"xmqqo9fgayb6.fsf@gitster-ct.c.googlers.com","subject":"Re: [RFC PATCH 4/6] sequencer.c: avoid empty statements at top level","fromName":"Beat Bolli","fromEmail":"dev+git@drbeat.li","sentAt":"2018-07-09T21:37:15Z","receivedAt":"2018-07-09T21:37:22Z","isPatch":true,"sender":{"key":"dev+git@drbeat.li","avatar":"https://avatars.githubusercontent.com/u/21444?v=4"},"body":"On 09.07.18 23:34, Junio C Hamano wrote:\n> Beat Bolli <dev+git@drbeat.li> writes:\n> \n>> The marco GIT_PATH_FUNC expands to a complete statement including the\n>> semicolon. Remove two extra trailing semicolons.\n> \n> Wait a bit.  The observation in the log message and the\n> implementation of GIT_PATH_FUNC() do not match.\n> \n>         #define GIT_PATH_FUNC(func, filename) \\\n>                 const char *func(void) \\\n>                 { \\\n>                         static char *ret; \\\n>                         if (!ret) \\\n>                                 ret = git_pathdup(filename); \\\n>                         return ret; \\\n>                 }\n> \n> The code generated does \"include semicolon\" but that is not why the\n> caller should place semicolon after the closing parens.  Perhaps\n> replace \"including the semicolon.\" with something else, like \", and\n> adding a semicolon after it not only is unnecessary but is wrong.\"\n> or soemthing like that?\n\nThis message is fixed in the non-RFC series that I sent at 19:25 UTC. I\nnoticed the error after the message from Philip Oakley.\n\nBeat\n"},{"id":"352034","messageId":"xmqqd0vwaxrq.fsf@gitster-ct.c.googlers.com","threadId":"48839","inReplyTo":"20180709192537.18564-1-dev+git@drbeat.li","subject":"Re: [PATCH 0/6] Compile cleanly in pedantic mode","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-07-09T21:45:45Z","receivedAt":"2018-07-09T21:45:51Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Beat Bolli <dev+git@drbeat.li> writes:\n\n> While developing 6aaded550 (\"builtin/config: work around an unsized\n> array forward declaration\", 2018-07-05), I have compiled Git with\n> CFLAGS=\"-std=c99 -pedantic\".\n\nNicely done.  \n\nWith these 6 patches and the USE_PARENCE_AROUND_GETTEXT_N hack, the\nforward decl of the unsized static array you dealt with separately\nbecomes the only remaining violation in the codebase, which is good.\n\nWill queue.  Thanks.\n"},{"id":"352036","messageId":"0306f303-81c1-af5a-4b4f-007f948dc563@drbeat.li","threadId":"48839","inReplyTo":"xmqqd0vwaxrq.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH 0/6] Compile cleanly in pedantic mode","fromName":"Beat Bolli","fromEmail":"dev+git@drbeat.li","sentAt":"2018-07-09T21:47:46Z","receivedAt":"2018-07-09T21:47:52Z","isPatch":true,"sender":{"key":"dev+git@drbeat.li","avatar":"https://avatars.githubusercontent.com/u/21444?v=4"},"body":"On 09.07.18 23:45, Junio C Hamano wrote:\n> Beat Bolli <dev+git@drbeat.li> writes:\n> \n>> While developing 6aaded550 (\"builtin/config: work around an unsized\n>> array forward declaration\", 2018-07-05), I have compiled Git with\n>> CFLAGS=\"-std=c99 -pedantic\".\n> \n> Nicely done.  \n> \n> With these 6 patches and the USE_PARENCE_AROUND_GETTEXT_N hack, the\n> forward decl of the unsized static array you dealt with separately\n> becomes the only remaining violation in the codebase, which is good.\n> \n> Will queue.  Thanks.\n\nThanks!\n\nBeat\n"},{"id":"352057","messageId":"20180710021558.GB10248@sigill.intra.peff.net","threadId":"48839","inReplyTo":"a3e6c187-74a5-1858-b207-b52d409b11be@drbeat.li","subject":"Re: [RFC PATCH 2/6] refs/refs-internal.h: avoid forward declaration of an enum","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-07-10T02:15:59Z","receivedAt":"2018-07-10T02:16:04Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Jul 09, 2018 at 09:30:12PM +0200, Beat Bolli wrote:\n\n> > Other than this minor quibble, the whole series looks good to me, modulo\n> > the existing review.\n> \n> Ooosp, I've just sent the non-RFC reroll without this change.\n> \n> Junio, would you squash this into [1/6] and [2/6], please (if you agree,\n> of course :-)\n\n:) I'd be happy with that squash, but not the end of the world if it\ndoesn't make it. Thanks for the whole series.\n\n-Peff\n"},{"id":"352064","messageId":"3f0ab3f42606d2251a6574d6fb45bf11@drbeat.li","threadId":"48839","inReplyTo":"xmqqd0vwaxrq.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH 0/6] Compile cleanly in pedantic mode","fromName":"Beat Bolli","fromEmail":"dev+git@drbeat.li","sentAt":"2018-07-10T07:34:48Z","receivedAt":"2018-07-10T07:34:55Z","isPatch":true,"sender":{"key":"dev+git@drbeat.li","avatar":"https://avatars.githubusercontent.com/u/21444?v=4"},"body":"Hi Junio\n\nAm 09.07.2018 23:45, schrieb Junio C Hamano:\n> Beat Bolli <dev+git@drbeat.li> writes:\n> \n>> While developing 6aaded550 (\"builtin/config: work around an unsized\n>> array forward declaration\", 2018-07-05), I have compiled Git with\n>> CFLAGS=\"-std=c99 -pedantic\".\n> \n> Nicely done.\n> \n> With these 6 patches and the USE_PARENCE_AROUND_GETTEXT_N hack, the\n> forward decl of the unsized static array you dealt with separately\n> becomes the only remaining violation in the codebase, which is good.\n> \n> Will queue.  Thanks.\n\nShould we add a \"pedantic\" flag to DEVOPTS that would simplify building \npedantically? It would also have to set USE_PARENS_AROUND_GETTEXT_N so \nas to not overwhelm the developer with too much output.\n\nBeat\n"},{"id":"352260","messageId":"xmqqin5l4w3r.fsf@gitster-ct.c.googlers.com","threadId":"48839","inReplyTo":"3f0ab3f42606d2251a6574d6fb45bf11@drbeat.li","subject":"Re: [PATCH 0/6] Compile cleanly in pedantic mode","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-07-11T15:42:48Z","receivedAt":"2018-07-11T15:42:53Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Beat Bolli <dev+git@drbeat.li> writes:\n\n> Should we add a \"pedantic\" flag to DEVOPTS that would simplify\n> building pedantically? It would also have to set\n> USE_PARENS_AROUND_GETTEXT_N so as to not overwhelm the developer with\n> too much output.\n\nThat may be something worth discussing before doing; I'd prefer to\nwait until these 6 patches, plus the unsized static array one you\ndid spearately, graduates to the 'master' branch.\n"},{"id":"352338","messageId":"nycvar.QRO.7.76.6.1807121524420.75@tvgsbejvaqbjf.bet","threadId":"48839","inReplyTo":"xmqqin5l4w3r.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH 0/6] Compile cleanly in pedantic mode","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-07-12T13:25:51Z","receivedAt":"2018-07-12T13:26:21Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 11 Jul 2018, Junio C Hamano wrote:\n\n> Beat Bolli <dev+git@drbeat.li> writes:\n> \n> > Should we add a \"pedantic\" flag to DEVOPTS that would simplify\n> > building pedantically? It would also have to set\n> > USE_PARENS_AROUND_GETTEXT_N so as to not overwhelm the developer with\n> > too much output.\n> \n> That may be something worth discussing before doing; I'd prefer to\n> wait until these 6 patches, plus the unsized static array one you\n> did spearately, graduates to the 'master' branch.\n\nIf this change to DEVOPTS was done as 7/7, then this would be very easy to\nguarantee (or as 2/2 if you want to graduate this here patch series faster\nthan the static array forward-declaration fix).\n\nJust an idea,\nDscho\n"},{"id":"352349","messageId":"xmqq36wo1mzd.fsf@gitster-ct.c.googlers.com","threadId":"48839","inReplyTo":"nycvar.QRO.7.76.6.1807121524420.75@tvgsbejvaqbjf.bet","subject":"Re: [PATCH 0/6] Compile cleanly in pedantic mode","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-07-12T15:40:22Z","receivedAt":"2018-07-12T15:40:26Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n\n>> That may be something worth discussing before doing; I'd prefer to\n>> wait until these 6 patches, plus the unsized static array one you\n>> did spearately, graduates to the 'master' branch.\n>\n> If this change to DEVOPTS was done as 7/7, then this would be very easy to\n> guarantee (or as 2/2 if you want to graduate this here patch series faster\n> than the static array forward-declaration fix).\n\nIt would guarantee that there is *no* period to discuss if it is too\ninconvenient for developers between the time these 6 plus 1 graduate\nto the master branch (and willing volunteers have chance to try it\nthemselves) and the change actuallly is forced upon the developers.\n\nThat is why I said I'd prefer to wait.\n\nA separate patch that depends on the 6 plus 1 topic that is kept in\n'next' longer than the main series is a possibility, though.\n"}]}