{"thread":{"id":"58579","subject":"[PATCH 1/5] CodingGuidelines: update for C99","startedAt":"2022-10-07T09:30:50Z","lastAt":"2022-10-11T15:46:42Z","messageCount":22,"participants":["Ævar Arnfjörð Bjarmason","Junio C Hamano","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":5},"messages":[{"id":"464352","messageId":"patch-1.5-5ea53989486-20221007T092505Z-avarab@gmail.com","threadId":"58579","inReplyTo":"cover-0.5-00000000000-20221007T092505Z-avarab@gmail.com","subject":"[PATCH 1/5] CodingGuidelines: update for C99","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-10-07T09:30:30Z","receivedAt":"2022-10-07T09:30:50Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Since 7bc341e21b5 (git-compat-util: add a test balloon for C99\nsupport, 2021-12-01) we've had a hard dependency on C99, but the prose\nin CodingGuidelines was written under the assumption that we were\nusing C89 with a few C99 features.\n\nAs the updated prose notes we'd still like to hold off on novel C99\nfeatures, but let's make it clear that we target that C version, and\nthen enumerate new C99 features that are safe to use.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n Documentation/CodingGuidelines | 8 ++++++--\n 1 file changed, 6 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines\nindex 9fca21cc5f9..386ca0a0d22 100644\n--- a/Documentation/CodingGuidelines\n+++ b/Documentation/CodingGuidelines\n@@ -204,10 +204,14 @@ For C programs:\n    by e.g. \"echo DEVELOPER=1 >>config.mak\".\n \n  - We try to support a wide range of C compilers to compile Git with,\n-   including old ones.  You should not use features from newer C\n+   including old ones.  As of Git v2.35.0 Git requires C99 (we check\n+   \"__STDC_VERSION__\"). You should not use features from a newer C\n    standard, even if your compiler groks them.\n \n-   There are a few exceptions to this guideline:\n+   New C99 features have been phased in gradually, if something's new\n+   in C99 but not used yet don't assume that it's safe to use, some\n+   compilers we target have only partial support for it. These are\n+   considered safe to use:\n \n    . since early 2012 with e1327023ea, we have been using an enum\n      definition whose last element is followed by a comma.  This, like\n-- \n2.38.0.971.ge79ff6d20e7\n\n"},{"id":"464353","messageId":"cover-0.5-00000000000-20221007T092505Z-avarab@gmail.com","threadId":"58579","inReplyTo":null,"subject":"[PATCH 0/5] CodingGuidelines: various C99 updates","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-10-07T09:30:29Z","receivedAt":"2022-10-07T09:30:51Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"This series:\n\n * Rephrases CodingGuidelines so that we note we're on C99, and then\n   lists exceptions and features we use. The previous prose assumed\n   C89 by default.\n\n   We still explicitly avoid opening the \"feel free to use an C99\n   feature\" floodgates.\n\n * Mentions that you can use dynamic C99 initializer elements. See the\n   recent discussion at\n   https://lore.kernel.org/git/221006.86a668r5mf.gmgdl@evledraar.gmail.com/\n\n * Allows us to use \"for (int i\". I didn't set out to (slightly) jump\n   the gun on this, but just pulling the trigger around ~20 days early\n   makes it easier to ...\n\n * ...add the natural follow-up section of C99 features you explicitly\n   shouldn't be using yet, to which I added the two cases I could\n   remember (in 4-5/5).\n\nÆvar Arnfjörð Bjarmason (5):\n  CodingGuidelines: update for C99\n  CodingGuidelines: mention dynamic C99 initializer elements\n  CodingGuidelines: allow declaring variables in for loops\n  CodingGuidelines: mention C99 features we can't use\n  CodingGuidelines: recommend against unportable C99 struct syntax\n\n Documentation/CodingGuidelines | 34 ++++++++++++++++++++++++----------\n revision.c                     |  7 -------\n t/helper/test-parse-options.c  |  3 +--\n 3 files changed, 25 insertions(+), 19 deletions(-)\n\n-- \n2.38.0.971.ge79ff6d20e7\n\n"},{"id":"464354","messageId":"patch-2.5-343ec223dff-20221007T092505Z-avarab@gmail.com","threadId":"58579","inReplyTo":"cover-0.5-00000000000-20221007T092505Z-avarab@gmail.com","subject":"[PATCH 2/5] CodingGuidelines: mention dynamic C99 initializer elements","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-10-07T09:30:31Z","receivedAt":"2022-10-07T09:30:54Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"The first use of variables in initializer elements appears to have\nbeen 2b6854c863a (Cleanup variables in cat-file, 2007-04-21) released\nwith v1.5.2.\n\nSome of those caused portability issues, and e.g. that \"cat-file\" use\nwas changed in 66dbfd55e38 (Rewrite dynamic structure initializations\nto runtime assignment, 2010-05-14) which went out with v1.7.2.\n\nBut curiously 66dbfd55e38 missed some of them, e.g. an archive.c use\nadded in d5f53d6d6f2 (archive: complain about path specs that don't\nmatch anything, 2009-12-12), and another one in merge-index.c (later\nbuiltin/merge-index.c) in 0077138cd9d (Simplify some instances of\nrun_command() by using run_command_v_opt()., 2009-06-08).\n\nAs far as I can tell there's been no point since 2b6854c863a in 2007\nwhere a compiler that didn't support this has been able to compile\ngit. Presumably 66dbfd55e38 was an attempt to make headway with wider\nportability that ultimately wasn't completed.\n\nIn any case, we are thoroughly reliant on this syntax at this point,\nso let's update the guidelines, see\nhttps://lore.kernel.org/git/xmqqy1tunjgp.fsf@gitster.g/ for the\ninitial discussion.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n Documentation/CodingGuidelines | 5 +++++\n 1 file changed, 5 insertions(+)\n\ndiff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines\nindex 386ca0a0d22..8afda28cfce 100644\n--- a/Documentation/CodingGuidelines\n+++ b/Documentation/CodingGuidelines\n@@ -213,6 +213,11 @@ For C programs:\n    compilers we target have only partial support for it. These are\n    considered safe to use:\n \n+   . since around 2007 with 2b6854c863a, we have been using\n+     initializer elements which are not computable at load time. E.g.:\n+\n+\tconst char *args[] = {\"constant\", variable, NULL};\n+\n    . since early 2012 with e1327023ea, we have been using an enum\n      definition whose last element is followed by a comma.  This, like\n      an array initializer that ends with a trailing comma, can be used\n-- \n2.38.0.971.ge79ff6d20e7\n\n"},{"id":"464355","messageId":"patch-3.5-80afc246666-20221007T092505Z-avarab@gmail.com","threadId":"58579","inReplyTo":"cover-0.5-00000000000-20221007T092505Z-avarab@gmail.com","subject":"[PATCH 3/5] CodingGuidelines: allow declaring variables in for loops","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-10-07T09:30:32Z","receivedAt":"2022-10-07T09:30:56Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Since 44ba10d6712 (revision: use C99 declaration of variable in for()\nloop, 2021-11-14) released with v2.35.0 we've had a variable declared\nwith in a for loop.\n\nSince then we've had inadvertent follow-ups to that with at least\ncb2607759e2 (merge-ort: store more specific conflict information,\n2022-06-18) released with v2.38.0.\n\nAs November 2022 is within the window of this upcoming release let's\nupdate the guideline to allow this, and revert the recent\n6983f4e3b20 (test-parse-options.c: don't use for loop initial\ndeclaration, 2022-09-05).\n\nIt's better to update the guidelines than to have back & forth churn\nlike that, we clearly don't have portability issues related to this\nsyntax.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n Documentation/CodingGuidelines | 10 ++--------\n revision.c                     |  7 -------\n t/helper/test-parse-options.c  |  3 +--\n 3 files changed, 3 insertions(+), 17 deletions(-)\n\ndiff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines\nindex 8afda28cfce..f9affc4050a 100644\n--- a/Documentation/CodingGuidelines\n+++ b/Documentation/CodingGuidelines\n@@ -232,18 +232,12 @@ For C programs:\n    . since early 2021 with 765dc168882, we have been using variadic\n      macros, mostly for printf-like trace and debug macros.\n \n-   These used to be forbidden, but we have not heard any breakage\n-   report, and they are assumed to be safe.\n+   . since late 2021 with 44ba10d6, we have had variables declared in\n+     the for loop \"for (int i = 0; i < 10; i++)\".\n \n  - Variables have to be declared at the beginning of the block, before\n    the first statement (i.e. -Wdeclaration-after-statement).\n \n- - Declaring a variable in the for loop \"for (int i = 0; i < 10; i++)\"\n-   is still not allowed in this codebase.  We are in the process of\n-   allowing it by waiting to see that 44ba10d6 (revision: use C99\n-   declaration of variable in for() loop, 2021-11-14) does not get\n-   complaints.  Let's revisit this around November 2022.\n-\n  - NULL pointers shall be written as NULL, not as 0.\n \n  - When declaring pointers, the star sides with the variable\ndiff --git a/revision.c b/revision.c\nindex 36e31942cee..8f2623b3b5a 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -47,13 +47,6 @@ static inline int want_ancestry(const struct rev_info *revs);\n void show_object_with_name(FILE *out, struct object *obj, const char *name)\n {\n \tfprintf(out, \"%s \", oid_to_hex(&obj->oid));\n-\t/*\n-\t * This \"for (const char *p = ...\" is made as a first step towards\n-\t * making use of such declarations elsewhere in our codebase.  If\n-\t * it causes compilation problems on your platform, please report\n-\t * it to the Git mailing list at git@vger.kernel.org. In the meantime,\n-\t * adding -std=gnu99 to CFLAGS may help if you are with older GCC.\n-\t */\n \tfor (const char *p = name; *p && *p != '\\n'; p++)\n \t\tfputc(*p, out);\n \tfputc('\\n', out);\ndiff --git a/t/helper/test-parse-options.c b/t/helper/test-parse-options.c\nindex 506835521a4..f8a62d892d9 100644\n--- a/t/helper/test-parse-options.c\n+++ b/t/helper/test-parse-options.c\n@@ -195,8 +195,7 @@ int cmd__parse_options(int argc, const char **argv)\n \n static void print_args(int argc, const char **argv)\n {\n-\tint i;\n-\tfor (i = 0; i < argc; i++)\n+\tfor (int i = 0; i < argc; i++)\n \t\tprintf(\"arg %02d: %s\\n\", i, argv[i]);\n }\n \n-- \n2.38.0.971.ge79ff6d20e7\n\n"},{"id":"464356","messageId":"patch-4.5-17c2a0d88e8-20221007T092505Z-avarab@gmail.com","threadId":"58579","inReplyTo":"cover-0.5-00000000000-20221007T092505Z-avarab@gmail.com","subject":"[PATCH 4/5] CodingGuidelines: mention C99 features we can't use","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-10-07T09:30:33Z","receivedAt":"2022-10-07T09:30:59Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"The C99 section of the CodingGuidelines is a good overview of what we\ncan use, but is sorely lacking in what we can't use. Something that\ncomes up occasionally is the portability of %z.\n\nPer [1] we couldn't use it for the longest time due to MSVC not\nsupporting it, but nowadays by requiring C99 we rely on the MSVC\nversion that does, but we can't use it yet because a C library that\nMinGW uses doesn't support it.\n\n1. https://lore.kernel.org/git/a67e0fd8-4a14-16c9-9b57-3430440ef93c@gmail.com/\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n Documentation/CodingGuidelines | 7 +++++++\n 1 file changed, 7 insertions(+)\n\ndiff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines\nindex f9affc4050a..893f960231f 100644\n--- a/Documentation/CodingGuidelines\n+++ b/Documentation/CodingGuidelines\n@@ -235,6 +235,13 @@ For C programs:\n    . since late 2021 with 44ba10d6, we have had variables declared in\n      the for loop \"for (int i = 0; i < 10; i++)\".\n \n+   New C99 features that we cannot use yet:\n+\n+   . %z and %zu as a printf() argument for a size_t (the %z being for\n+     the POSIX-specific ssize_t). Instead you should use\n+     printf(\"%\"PRIuMAX, (uintmax_t)v); These days the MSVC version we\n+     rely on supports %z, but the C library used by MinGW does not.\n+\n  - Variables have to be declared at the beginning of the block, before\n    the first statement (i.e. -Wdeclaration-after-statement).\n \n-- \n2.38.0.971.ge79ff6d20e7\n\n"},{"id":"464357","messageId":"patch-5.5-fc02ae52f4a-20221007T092505Z-avarab@gmail.com","threadId":"58579","inReplyTo":"cover-0.5-00000000000-20221007T092505Z-avarab@gmail.com","subject":"[PATCH 5/5] CodingGuidelines: recommend against unportable C99 struct syntax","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-10-07T09:30:34Z","receivedAt":"2022-10-07T09:31:01Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Per 33665d98e6b (reftable: make assignments portable to AIX xlc\nv12.01, 2022-03-28) forms like \".a.b = *c\" can be replaced by using\n\".a = { .b = *c }\" instead.\n\nWe'll probably allow these sooner than later, but since the workaround\nis trivial let's note it among the C99 features we'd like to hold off\non for now.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n Documentation/CodingGuidelines | 4 ++++\n 1 file changed, 4 insertions(+)\n\ndiff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines\nindex 893f960231f..65b608ca0a2 100644\n--- a/Documentation/CodingGuidelines\n+++ b/Documentation/CodingGuidelines\n@@ -242,6 +242,10 @@ For C programs:\n      printf(\"%\"PRIuMAX, (uintmax_t)v); These days the MSVC version we\n      rely on supports %z, but the C library used by MinGW does not.\n \n+   . Shorthand like \".a.b = *c\" in struct assignments is known to trip\n+     up an older IBM XLC version, use \".a = { .b = *c }\" instead. See\n+     the 33665d98e6b portability fix from mid-2022.\n+\n  - Variables have to be declared at the beginning of the block, before\n    the first statement (i.e. -Wdeclaration-after-statement).\n \n-- \n2.38.0.971.ge79ff6d20e7\n\n"},{"id":"464389","messageId":"xmqqk05bfrk7.fsf@gitster.g","threadId":"58579","inReplyTo":"cover-0.5-00000000000-20221007T092505Z-avarab@gmail.com","subject":"Re: [PATCH 0/5] CodingGuidelines: various C99 updates","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-10-07T17:54:00Z","receivedAt":"2022-10-07T17:54:08Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:\n\n> This series:\n>\n>  * Rephrases CodingGuidelines so that we note we're on C99, and then\n>    lists exceptions and features we use. The previous prose assumed\n>    C89 by default.\n>\n>    We still explicitly avoid opening the \"feel free to use an C99\n>    feature\" floodgates.\n\nThe above contradicts with each other.  A sensible position to\nsupport the \"we do not open the floodgate\" is that when in doubt\nstick to C89 but use C99 features that are explicitly allowed.\n\n>  * Mentions that you can use dynamic C99 initializer elements. See the\n>    recent discussion at\n>    https://lore.kernel.org/git/221006.86a668r5mf.gmgdl@evledraar.gmail.com/\n\nGood.\n\n>  * Allows us to use \"for (int i\". I didn't set out to (slightly) jump\n>    the gun on this, but just pulling the trigger around ~20 days early\n>    makes it easier to ...\n\nThis is a welcome change.  As anything this set of patches won't\nbecome reality in any released version until mid December anyway,\nthis is the cycle to \"revisit around November 2022\".\n\n>  * ...add the natural follow-up section of C99 features you explicitly\n>    shouldn't be using yet, to which I added the two cases I could\n>    remember (in 4-5/5).\n\nAnd we do not have to say we do not use these from C99 if our base\nis C89, with explicitly allowed features from C99.\n"},{"id":"464391","messageId":"xmqqa667fqvt.fsf@gitster.g","threadId":"58579","inReplyTo":"patch-1.5-5ea53989486-20221007T092505Z-avarab@gmail.com","subject":"Re: [PATCH 1/5] CodingGuidelines: update for C99","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-10-07T18:08:38Z","receivedAt":"2022-10-07T18:10:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:\n\n> Since 7bc341e21b5 (git-compat-util: add a test balloon for C99\n> support, 2021-12-01) we've had a hard dependency on C99, but the prose\n> in CodingGuidelines was written under the assumption that we were\n> using C89 with a few C99 features.\n>\n> As the updated prose notes we'd still like to hold off on novel C99\n> features, but let's make it clear that we target that C version, and\n> then enumerate new C99 features that are safe to use.\n>\n> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n> ---\n>  Documentation/CodingGuidelines | 8 ++++++--\n>  1 file changed, 6 insertions(+), 2 deletions(-)\n>\n> diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines\n> index 9fca21cc5f9..386ca0a0d22 100644\n> --- a/Documentation/CodingGuidelines\n> +++ b/Documentation/CodingGuidelines\n> @@ -204,10 +204,14 @@ For C programs:\n>     by e.g. \"echo DEVELOPER=1 >>config.mak\".\n>  \n>   - We try to support a wide range of C compilers to compile Git with,\n> -   including old ones.  You should not use features from newer C\n> +   including old ones.  As of Git v2.35.0 Git requires C99 (we check\n> +   \"__STDC_VERSION__\"). You should not use features from a newer C\n>     standard, even if your compiler groks them.\n>  \n> -   There are a few exceptions to this guideline:\n> +   New C99 features have been phased in gradually, if something's new\n> +   in C99 but not used yet don't assume that it's safe to use, some\n> +   compilers we target have only partial support for it. These are\n> +   considered safe to use:\n>  \n>     . since early 2012 with e1327023ea, we have been using an enum\n>       definition whose last element is followed by a comma.  This, like\n\nLooking good.\n"},{"id":"464392","messageId":"xmqq5ygvfqp3.fsf@gitster.g","threadId":"58579","inReplyTo":"patch-3.5-80afc246666-20221007T092505Z-avarab@gmail.com","subject":"Re: [PATCH 3/5] CodingGuidelines: allow declaring variables in for loops","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-10-07T18:12:40Z","receivedAt":"2022-10-07T18:12:49Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:\n\n> diff --git a/revision.c b/revision.c\n> index 36e31942cee..8f2623b3b5a 100644\n> --- a/revision.c\n> +++ b/revision.c\n> @@ -47,13 +47,6 @@ static inline int want_ancestry(const struct rev_info *revs);\n>  void show_object_with_name(FILE *out, struct object *obj, const char *name)\n>  {\n>  \tfprintf(out, \"%s \", oid_to_hex(&obj->oid));\n> -\t/*\n> -\t * This \"for (const char *p = ...\" is made as a first step towards\n> -\t * making use of such declarations elsewhere in our codebase.  If\n> -\t * it causes compilation problems on your platform, please report\n> -\t * it to the Git mailing list at git@vger.kernel.org. In the meantime,\n> -\t * adding -std=gnu99 to CFLAGS may help if you are with older GCC.\n> -\t */\n>  \tfor (const char *p = name; *p && *p != '\\n'; p++)\n>  \t\tfputc(*p, out);\n>  \tfputc('\\n', out);\n\nGood.  Thanks for not forgetting to remove this.\n\n> diff --git a/t/helper/test-parse-options.c b/t/helper/test-parse-options.c\n> index 506835521a4..f8a62d892d9 100644\n> --- a/t/helper/test-parse-options.c\n> +++ b/t/helper/test-parse-options.c\n> @@ -195,8 +195,7 @@ int cmd__parse_options(int argc, const char **argv)\n>  \n>  static void print_args(int argc, const char **argv)\n>  {\n> -\tint i;\n> -\tfor (i = 0; i < argc; i++)\n> +\tfor (int i = 0; i < argc; i++)\n>  \t\tprintf(\"arg %02d: %s\\n\", i, argv[i]);\n>  }\n\nThis does not belong to this patch.  If the current code as written\nis acceptable, it is not worth patch churn to rewrite it.\n"},{"id":"464393","messageId":"xmqq1qrjfqn6.fsf@gitster.g","threadId":"58579","inReplyTo":"patch-4.5-17c2a0d88e8-20221007T092505Z-avarab@gmail.com","subject":"Re: [PATCH 4/5] CodingGuidelines: mention C99 features we can't use","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-10-07T18:13:49Z","receivedAt":"2022-10-07T18:13:54Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:\n\n> The C99 section of the CodingGuidelines is a good overview of what we\n> can use, but is sorely lacking in what we can't use. Something that\n> comes up occasionally is the portability of %z.\n>\n> Per [1] we couldn't use it for the longest time due to MSVC not\n> supporting it, but nowadays by requiring C99 we rely on the MSVC\n> version that does, but we can't use it yet because a C library that\n> MinGW uses doesn't support it.\n>\n> 1. https://lore.kernel.org/git/a67e0fd8-4a14-16c9-9b57-3430440ef93c@gmail.com/\n>\n> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n> ---\n>  Documentation/CodingGuidelines | 7 +++++++\n>  1 file changed, 7 insertions(+)\n>\n> diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines\n> index f9affc4050a..893f960231f 100644\n> --- a/Documentation/CodingGuidelines\n> +++ b/Documentation/CodingGuidelines\n> @@ -235,6 +235,13 @@ For C programs:\n>     . since late 2021 with 44ba10d6, we have had variables declared in\n>       the for loop \"for (int i = 0; i < 10; i++)\".\n>  \n> +   New C99 features that we cannot use yet:\n> +\n> +   . %z and %zu as a printf() argument for a size_t (the %z being for\n> +     the POSIX-specific ssize_t). Instead you should use\n> +     printf(\"%\"PRIuMAX, (uintmax_t)v); These days the MSVC version we\n> +     rely on supports %z, but the C library used by MinGW does not.\n> +\n\nGood.  s/; These days/; these days/, though.\n\n"},{"id":"464394","messageId":"xmqqwn9bebzs.fsf@gitster.g","threadId":"58579","inReplyTo":"xmqqk05bfrk7.fsf@gitster.g","subject":"Re: [PATCH 0/5] CodingGuidelines: various C99 updates","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-10-07T18:15:35Z","receivedAt":"2022-10-07T18:15:46Z","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> Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:\n>\n>> This series:\n>>\n>>  * Rephrases CodingGuidelines so that we note we're on C99, and then\n>>    lists exceptions and features we use. The previous prose assumed\n>>    C89 by default.\n>>\n>>    We still explicitly avoid opening the \"feel free to use an C99\n>>    feature\" floodgates.\n>\n> The above contradicts with each other.  A sensible position to\n> support the \"we do not open the floodgate\" is that when in doubt\n> stick to C89 but use C99 features that are explicitly allowed.\n\nAh, I think I misread the intention, which 1/5 clarified.  I think\n\"even though we require your CC claim to support C99, you are\nexpected to stick to C89 plus those features that are explicitly\nallowed\" is a very sensible thing to day.\n"},{"id":"464530","messageId":"20221010203800.2154698-1-gitster@pobox.com","threadId":"58579","inReplyTo":"cover-0.5-00000000000-20221007T092505Z-avarab@gmail.com","subject":"[PATCH v2 0/5] CodingGUidelines: C99 updates","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-10-10T20:37:55Z","receivedAt":"2022-10-10T20:38:25Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"As this is otherwise clearly good series, I got sick of waiting for\na trivial update, so here is what I did to update them myself.\n\nUpdates are in patches 3 and 4.  We promised to revisit the issue of\nallowing \"for (int i = 0; ...\" in November this year, so we are\nmerely \"proposing\" to go ahead and allow it with this patch, and\nwill merge only after gaining consensus.  The wording has been\nredone to avoid sounding like it is a fait accompli.\n\nPatch 4 has been updated to avoid starting the latter clause with a\ncapitalized word in two independent clauses joined with a semicolon.\n\n\nÆvar Arnfjörð Bjarmason (5):\n  CodingGuidelines: update for C99\n  CodingGuidelines: mention dynamic C99 initializer elements\n  CodingGuidelines: allow declaring variables in for loops\n  CodingGuidelines: mention C99 features we can't use\n  CodingGuidelines: recommend against unportable C99 struct syntax\n\n Documentation/CodingGuidelines | 34 ++++++++++++++++++++++++----------\n revision.c                     |  7 -------\n 2 files changed, 24 insertions(+), 17 deletions(-)\n\nRange-diff against v1:\n1:  3e3dd9cbd5 = 1:  3e3dd9cbd5 CodingGuidelines: update for C99\n2:  fb6c58ef52 = 2:  fb6c58ef52 CodingGuidelines: mention dynamic C99 initializer elements\n3:  719e235f9f ! 3:  f3237c88f8 CodingGuidelines: allow declaring variables in for loops\n    @@ Commit message\n         cb2607759e2 (merge-ort: store more specific conflict information,\n         2022-06-18) released with v2.38.0.\n     \n    -    As November 2022 is within the window of this upcoming release let's\n    -    update the guideline to allow this, and revert the recent\n    -    6983f4e3b20 (test-parse-options.c: don't use for loop initial\n    -    declaration, 2022-09-05).\n    -\n    -    It's better to update the guidelines than to have back & forth churn\n    -    like that, we clearly don't have portability issues related to this\n    -    syntax.\n    +    As November 2022 is within the window of this upcoming release,\n    +    let's update the guideline to allow this.  We can have the promised\n    +    \"revisit\" discussion while this patch cooks, and drop it if it turns\n    +    out that it is still premature, which is not expected to happen at\n    +    this moment.\n     \n         Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n         Signed-off-by: Junio C Hamano <gitster@pobox.com>\n    @@ revision.c: static inline int want_ancestry(const struct rev_info *revs);\n      \tfor (const char *p = name; *p && *p != '\\n'; p++)\n      \t\tfputc(*p, out);\n      \tfputc('\\n', out);\n    -\n    - ## t/helper/test-parse-options.c ##\n    -@@ t/helper/test-parse-options.c: int cmd__parse_options(int argc, const char **argv)\n    - \n    - static void print_args(int argc, const char **argv)\n    - {\n    --\tint i;\n    --\tfor (i = 0; i < argc; i++)\n    -+\tfor (int i = 0; i < argc; i++)\n    - \t\tprintf(\"arg %02d: %s\\n\", i, argv[i]);\n    - }\n    - \n4:  3d1edfd825 ! 4:  9d2479220d CodingGuidelines: mention C99 features we can't use\n    @@ Documentation/CodingGuidelines: For C programs:\n     +\n     +   . %z and %zu as a printf() argument for a size_t (the %z being for\n     +     the POSIX-specific ssize_t). Instead you should use\n    -+     printf(\"%\"PRIuMAX, (uintmax_t)v); These days the MSVC version we\n    ++     printf(\"%\"PRIuMAX, (uintmax_t)v).  These days the MSVC version we\n     +     rely on supports %z, but the C library used by MinGW does not.\n     +\n       - Variables have to be declared at the beginning of the block, before\n5:  139b0f8cfd ! 5:  ef22b1756a CodingGuidelines: recommend against unportable C99 struct syntax\n    @@ Commit message\n     \n      ## Documentation/CodingGuidelines ##\n     @@ Documentation/CodingGuidelines: For C programs:\n    -      printf(\"%\"PRIuMAX, (uintmax_t)v); These days the MSVC version we\n    +      printf(\"%\"PRIuMAX, (uintmax_t)v).  These days the MSVC version we\n           rely on supports %z, but the C library used by MinGW does not.\n      \n     +   . Shorthand like \".a.b = *c\" in struct assignments is known to trip\n\n-- \n2.38.0-167-gf9a88ca9e9\n\n"},{"id":"464531","messageId":"20221010203800.2154698-2-gitster@pobox.com","threadId":"58579","inReplyTo":"20221010203800.2154698-1-gitster@pobox.com","subject":"[PATCH v2 1/5] CodingGuidelines: update for C99","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-10-10T20:37:56Z","receivedAt":"2022-10-10T20:38:26Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n\nSince 7bc341e21b5 (git-compat-util: add a test balloon for C99\nsupport, 2021-12-01) we've had a hard dependency on C99, but the prose\nin CodingGuidelines was written under the assumption that we were\nusing C89 with a few C99 features.\n\nAs the updated prose notes we'd still like to hold off on novel C99\nfeatures, but let's make it clear that we target that C version, and\nthen enumerate new C99 features that are safe to use.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n Documentation/CodingGuidelines | 8 ++++++--\n 1 file changed, 6 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines\nindex 9fca21cc5f..386ca0a0d2 100644\n--- a/Documentation/CodingGuidelines\n+++ b/Documentation/CodingGuidelines\n@@ -204,10 +204,14 @@ For C programs:\n    by e.g. \"echo DEVELOPER=1 >>config.mak\".\n \n  - We try to support a wide range of C compilers to compile Git with,\n-   including old ones.  You should not use features from newer C\n+   including old ones.  As of Git v2.35.0 Git requires C99 (we check\n+   \"__STDC_VERSION__\"). You should not use features from a newer C\n    standard, even if your compiler groks them.\n \n-   There are a few exceptions to this guideline:\n+   New C99 features have been phased in gradually, if something's new\n+   in C99 but not used yet don't assume that it's safe to use, some\n+   compilers we target have only partial support for it. These are\n+   considered safe to use:\n \n    . since early 2012 with e1327023ea, we have been using an enum\n      definition whose last element is followed by a comma.  This, like\n-- \n2.38.0-167-gf9a88ca9e9\n\n"},{"id":"464532","messageId":"20221010203800.2154698-3-gitster@pobox.com","threadId":"58579","inReplyTo":"20221010203800.2154698-1-gitster@pobox.com","subject":"[PATCH v2 2/5] CodingGuidelines: mention dynamic C99 initializer elements","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-10-10T20:37:57Z","receivedAt":"2022-10-10T20:38:28Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n\nThe first use of variables in initializer elements appears to have\nbeen 2b6854c863a (Cleanup variables in cat-file, 2007-04-21) released\nwith v1.5.2.\n\nSome of those caused portability issues, and e.g. that \"cat-file\" use\nwas changed in 66dbfd55e38 (Rewrite dynamic structure initializations\nto runtime assignment, 2010-05-14) which went out with v1.7.2.\n\nBut curiously 66dbfd55e38 missed some of them, e.g. an archive.c use\nadded in d5f53d6d6f2 (archive: complain about path specs that don't\nmatch anything, 2009-12-12), and another one in merge-index.c (later\nbuiltin/merge-index.c) in 0077138cd9d (Simplify some instances of\nrun_command() by using run_command_v_opt()., 2009-06-08).\n\nAs far as I can tell there's been no point since 2b6854c863a in 2007\nwhere a compiler that didn't support this has been able to compile\ngit. Presumably 66dbfd55e38 was an attempt to make headway with wider\nportability that ultimately wasn't completed.\n\nIn any case, we are thoroughly reliant on this syntax at this point,\nso let's update the guidelines, see\nhttps://lore.kernel.org/git/xmqqy1tunjgp.fsf@gitster.g/ for the\ninitial discussion.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n Documentation/CodingGuidelines | 5 +++++\n 1 file changed, 5 insertions(+)\n\ndiff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines\nindex 386ca0a0d2..8afda28cfc 100644\n--- a/Documentation/CodingGuidelines\n+++ b/Documentation/CodingGuidelines\n@@ -213,6 +213,11 @@ For C programs:\n    compilers we target have only partial support for it. These are\n    considered safe to use:\n \n+   . since around 2007 with 2b6854c863a, we have been using\n+     initializer elements which are not computable at load time. E.g.:\n+\n+\tconst char *args[] = {\"constant\", variable, NULL};\n+\n    . since early 2012 with e1327023ea, we have been using an enum\n      definition whose last element is followed by a comma.  This, like\n      an array initializer that ends with a trailing comma, can be used\n-- \n2.38.0-167-gf9a88ca9e9\n\n"},{"id":"464533","messageId":"20221010203800.2154698-4-gitster@pobox.com","threadId":"58579","inReplyTo":"20221010203800.2154698-1-gitster@pobox.com","subject":"[PATCH v2 3/5] CodingGuidelines: allow declaring variables in for loops","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-10-10T20:37:58Z","receivedAt":"2022-10-10T20:38:29Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n\nSince 44ba10d6712 (revision: use C99 declaration of variable in for()\nloop, 2021-11-14) released with v2.35.0 we've had a variable declared\nwith in a for loop.\n\nSince then we've had inadvertent follow-ups to that with at least\ncb2607759e2 (merge-ort: store more specific conflict information,\n2022-06-18) released with v2.38.0.\n\nAs November 2022 is within the window of this upcoming release,\nlet's update the guideline to allow this.  We can have the promised\n\"revisit\" discussion while this patch cooks, and drop it if it turns\nout that it is still premature, which is not expected to happen at\nthis moment.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n Documentation/CodingGuidelines | 10 ++--------\n revision.c                     |  7 -------\n 2 files changed, 2 insertions(+), 15 deletions(-)\n\ndiff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines\nindex 8afda28cfc..f9affc4050 100644\n--- a/Documentation/CodingGuidelines\n+++ b/Documentation/CodingGuidelines\n@@ -232,18 +232,12 @@ For C programs:\n    . since early 2021 with 765dc168882, we have been using variadic\n      macros, mostly for printf-like trace and debug macros.\n \n-   These used to be forbidden, but we have not heard any breakage\n-   report, and they are assumed to be safe.\n+   . since late 2021 with 44ba10d6, we have had variables declared in\n+     the for loop \"for (int i = 0; i < 10; i++)\".\n \n  - Variables have to be declared at the beginning of the block, before\n    the first statement (i.e. -Wdeclaration-after-statement).\n \n- - Declaring a variable in the for loop \"for (int i = 0; i < 10; i++)\"\n-   is still not allowed in this codebase.  We are in the process of\n-   allowing it by waiting to see that 44ba10d6 (revision: use C99\n-   declaration of variable in for() loop, 2021-11-14) does not get\n-   complaints.  Let's revisit this around November 2022.\n-\n  - NULL pointers shall be written as NULL, not as 0.\n \n  - When declaring pointers, the star sides with the variable\ndiff --git a/revision.c b/revision.c\nindex 36e31942ce..8f2623b3b5 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -47,13 +47,6 @@ static inline int want_ancestry(const struct rev_info *revs);\n void show_object_with_name(FILE *out, struct object *obj, const char *name)\n {\n \tfprintf(out, \"%s \", oid_to_hex(&obj->oid));\n-\t/*\n-\t * This \"for (const char *p = ...\" is made as a first step towards\n-\t * making use of such declarations elsewhere in our codebase.  If\n-\t * it causes compilation problems on your platform, please report\n-\t * it to the Git mailing list at git@vger.kernel.org. In the meantime,\n-\t * adding -std=gnu99 to CFLAGS may help if you are with older GCC.\n-\t */\n \tfor (const char *p = name; *p && *p != '\\n'; p++)\n \t\tfputc(*p, out);\n \tfputc('\\n', out);\n-- \n2.38.0-167-gf9a88ca9e9\n\n"},{"id":"464534","messageId":"20221010203800.2154698-5-gitster@pobox.com","threadId":"58579","inReplyTo":"20221010203800.2154698-1-gitster@pobox.com","subject":"[PATCH v2 4/5] CodingGuidelines: mention C99 features we can't use","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-10-10T20:37:59Z","receivedAt":"2022-10-10T20:38:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n\nThe C99 section of the CodingGuidelines is a good overview of what we\ncan use, but is sorely lacking in what we can't use. Something that\ncomes up occasionally is the portability of %z.\n\nPer [1] we couldn't use it for the longest time due to MSVC not\nsupporting it, but nowadays by requiring C99 we rely on the MSVC\nversion that does, but we can't use it yet because a C library that\nMinGW uses doesn't support it.\n\n1. https://lore.kernel.org/git/a67e0fd8-4a14-16c9-9b57-3430440ef93c@gmail.com/\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n Documentation/CodingGuidelines | 7 +++++++\n 1 file changed, 7 insertions(+)\n\ndiff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines\nindex f9affc4050..9598b45f7e 100644\n--- a/Documentation/CodingGuidelines\n+++ b/Documentation/CodingGuidelines\n@@ -235,6 +235,13 @@ For C programs:\n    . since late 2021 with 44ba10d6, we have had variables declared in\n      the for loop \"for (int i = 0; i < 10; i++)\".\n \n+   New C99 features that we cannot use yet:\n+\n+   . %z and %zu as a printf() argument for a size_t (the %z being for\n+     the POSIX-specific ssize_t). Instead you should use\n+     printf(\"%\"PRIuMAX, (uintmax_t)v).  These days the MSVC version we\n+     rely on supports %z, but the C library used by MinGW does not.\n+\n  - Variables have to be declared at the beginning of the block, before\n    the first statement (i.e. -Wdeclaration-after-statement).\n \n-- \n2.38.0-167-gf9a88ca9e9\n\n"},{"id":"464535","messageId":"20221010203800.2154698-6-gitster@pobox.com","threadId":"58579","inReplyTo":"20221010203800.2154698-1-gitster@pobox.com","subject":"[PATCH v2 5/5] CodingGuidelines: recommend against unportable C99 struct syntax","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-10-10T20:38:00Z","receivedAt":"2022-10-10T20:38:34Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n\nPer 33665d98e6b (reftable: make assignments portable to AIX xlc\nv12.01, 2022-03-28) forms like \".a.b = *c\" can be replaced by using\n\".a = { .b = *c }\" instead.\n\nWe'll probably allow these sooner than later, but since the workaround\nis trivial let's note it among the C99 features we'd like to hold off\non for now.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n Documentation/CodingGuidelines | 4 ++++\n 1 file changed, 4 insertions(+)\n\ndiff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines\nindex 9598b45f7e..cbe0377699 100644\n--- a/Documentation/CodingGuidelines\n+++ b/Documentation/CodingGuidelines\n@@ -242,6 +242,10 @@ For C programs:\n      printf(\"%\"PRIuMAX, (uintmax_t)v).  These days the MSVC version we\n      rely on supports %z, but the C library used by MinGW does not.\n \n+   . Shorthand like \".a.b = *c\" in struct assignments is known to trip\n+     up an older IBM XLC version, use \".a = { .b = *c }\" instead. See\n+     the 33665d98e6b portability fix from mid-2022.\n+\n  - Variables have to be declared at the beginning of the block, before\n    the first statement (i.e. -Wdeclaration-after-statement).\n \n-- \n2.38.0-167-gf9a88ca9e9\n\n"},{"id":"464548","messageId":"Y0S0Tpn/uCjG9J61@coredump.intra.peff.net","threadId":"58579","inReplyTo":"20221010203800.2154698-6-gitster@pobox.com","subject":"Re: [PATCH v2 5/5] CodingGuidelines: recommend against unportable C99 struct syntax","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2022-10-11T00:09:50Z","receivedAt":"2022-10-11T00:11:19Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Oct 10, 2022 at 01:38:00PM -0700, Junio C Hamano wrote:\n\n> diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines\n> index 9598b45f7e..cbe0377699 100644\n> --- a/Documentation/CodingGuidelines\n> +++ b/Documentation/CodingGuidelines\n> @@ -242,6 +242,10 @@ For C programs:\n>       printf(\"%\"PRIuMAX, (uintmax_t)v).  These days the MSVC version we\n>       rely on supports %z, but the C library used by MinGW does not.\n>  \n> +   . Shorthand like \".a.b = *c\" in struct assignments is known to trip\n> +     up an older IBM XLC version, use \".a = { .b = *c }\" instead. See\n> +     the 33665d98e6b portability fix from mid-2022.\n\nFWIW, the use of the word \"assignment\" here left me scratching my head.\nReading 33665d98e6b, it is about struct initialization.\n\nC99 does allow assignment from a compound literal, like:\n\n  foo = (struct some_struct){ .a = 1, .b = 2 };\n\nbut I don't think we have any examples in the current code (and I think\nwe'd consider it a different feature for purposes of testing a weather\nballoon).\n\n-Peff\n"},{"id":"464555","messageId":"xmqqlepnxl1f.fsf@gitster.g","threadId":"58579","inReplyTo":"Y0S0Tpn/uCjG9J61@coredump.intra.peff.net","subject":"Re: [PATCH v2 5/5] CodingGuidelines: recommend against unportable C99 struct syntax","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-10-11T00:26:36Z","receivedAt":"2022-10-11T00:26:44Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> On Mon, Oct 10, 2022 at 01:38:00PM -0700, Junio C Hamano wrote:\n>\n>> diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines\n>> index 9598b45f7e..cbe0377699 100644\n>> --- a/Documentation/CodingGuidelines\n>> +++ b/Documentation/CodingGuidelines\n>> @@ -242,6 +242,10 @@ For C programs:\n>>       printf(\"%\"PRIuMAX, (uintmax_t)v).  These days the MSVC version we\n>>       rely on supports %z, but the C library used by MinGW does not.\n>>  \n>> +   . Shorthand like \".a.b = *c\" in struct assignments is known to trip\n>> +     up an older IBM XLC version, use \".a = { .b = *c }\" instead. See\n>> +     the 33665d98e6b portability fix from mid-2022.\n>\n> FWIW, the use of the word \"assignment\" here left me scratching my head.\n> Reading 33665d98e6b, it is about struct initialization.\n\nThanks, I missed that confusion in the new description.  Perhaps\nanother round of reroll would make the series polished enough?\n"},{"id":"464566","messageId":"Y0S7OO4ecpvsnbtT@coredump.intra.peff.net","threadId":"58579","inReplyTo":"xmqqlepnxl1f.fsf@gitster.g","subject":"Re: [PATCH v2 5/5] CodingGuidelines: recommend against unportable C99 struct syntax","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2022-10-11T00:39:20Z","receivedAt":"2022-10-11T00:39:25Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Oct 10, 2022 at 05:26:36PM -0700, Junio C Hamano wrote:\n\n> Jeff King <peff@peff.net> writes:\n> \n> > On Mon, Oct 10, 2022 at 01:38:00PM -0700, Junio C Hamano wrote:\n> >\n> >> diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines\n> >> index 9598b45f7e..cbe0377699 100644\n> >> --- a/Documentation/CodingGuidelines\n> >> +++ b/Documentation/CodingGuidelines\n> >> @@ -242,6 +242,10 @@ For C programs:\n> >>       printf(\"%\"PRIuMAX, (uintmax_t)v).  These days the MSVC version we\n> >>       rely on supports %z, but the C library used by MinGW does not.\n> >>  \n> >> +   . Shorthand like \".a.b = *c\" in struct assignments is known to trip\n> >> +     up an older IBM XLC version, use \".a = { .b = *c }\" instead. See\n> >> +     the 33665d98e6b portability fix from mid-2022.\n> >\n> > FWIW, the use of the word \"assignment\" here left me scratching my head.\n> > Reading 33665d98e6b, it is about struct initialization.\n> \n> Thanks, I missed that confusion in the new description.  Perhaps\n> another round of reroll would make the series polished enough?\n\nI read through the rest of it fairly lightly, but I didn't see have any\nother complaints (and the overall goal of documenting our use of\ncompiler features is a great one).\n\n-Peff\n"},{"id":"464594","messageId":"221011.86bkqipxpv.gmgdl@evledraar.gmail.com","threadId":"58579","inReplyTo":"xmqqlepnxl1f.fsf@gitster.g","subject":"Re: [PATCH v2 5/5] CodingGuidelines: recommend against unportable C99 struct syntax","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-10-11T08:30:34Z","receivedAt":"2022-10-11T08:32:24Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Mon, Oct 10 2022, Junio C Hamano wrote:\n\n> Jeff King <peff@peff.net> writes:\n>\n>> On Mon, Oct 10, 2022 at 01:38:00PM -0700, Junio C Hamano wrote:\n>>\n>>> diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines\n>>> index 9598b45f7e..cbe0377699 100644\n>>> --- a/Documentation/CodingGuidelines\n>>> +++ b/Documentation/CodingGuidelines\n>>> @@ -242,6 +242,10 @@ For C programs:\n>>>       printf(\"%\"PRIuMAX, (uintmax_t)v).  These days the MSVC version we\n>>>       rely on supports %z, but the C library used by MinGW does not.\n>>>  \n>>> +   . Shorthand like \".a.b = *c\" in struct assignments is known to trip\n>>> +     up an older IBM XLC version, use \".a = { .b = *c }\" instead. See\n>>> +     the 33665d98e6b portability fix from mid-2022.\n>>\n>> FWIW, the use of the word \"assignment\" here left me scratching my head.\n>> Reading 33665d98e6b, it is about struct initialization.\n>\n> Thanks, I missed that confusion in the new description.  Perhaps\n> another round of reroll would make the series polished enough?\n\nI could re-roll it, but I also see you extensively fixed it up v.s. my\nversion. I think a re-roll here would just be\ns/assignments/initializations/, so if you wanted to squash that in to\nyour already extensive squashes...\n\n...or I could also re-roll it, up to you. Just let me know what you'd\nprefer, thanks!\n"},{"id":"464616","messageId":"xmqqh70awgiv.fsf@gitster.g","threadId":"58579","inReplyTo":"221011.86bkqipxpv.gmgdl@evledraar.gmail.com","subject":"Re: [PATCH v2 5/5] CodingGuidelines: recommend against unportable C99 struct syntax","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-10-11T15:01:44Z","receivedAt":"2022-10-11T15:46:42Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:\n\n>>> FWIW, the use of the word \"assignment\" here left me scratching my head.\n>>> Reading 33665d98e6b, it is about struct initialization.\n>>\n>> Thanks, I missed that confusion in the new description.  Perhaps\n>> another round of reroll would make the series polished enough?\n>\n> I could re-roll it, but I also see you extensively fixed it up v.s. my\n> version. I think a re-roll here would just be\n> s/assignments/initializations/, so if you wanted to squash that in to\n> your already extensive squashes...\n\nI do not think there has been that much squashing.  Unless I took a\nwrong range-diff in [0/5] of the series, the fix-ups are to reword\nthe proposed log message for [3/5] and to remove an unrelated hunk\nthat does \"now we allow it, let's use it\" which is better done\noutside the topic with its own justification (and \"it once used to\nbe there\" is not a good justification) in [3/5], plus a three-byte\ngrammofix in [4/5].\n\nSo throwing \"assignments\" -> \"initializations\" into the mix is not\ntoo much work for me.\n\nUnless I forget, that is ;-)\n"}]}