{"thread":{"id":"52326","subject":"[PATCH 1/1] git-compat-util.h: drop the `PRIuMAX` definition","startedAt":"2019-11-24T13:09:35Z","lastAt":"2019-11-25T09:34:03Z","messageCount":8,"participants":["Hariom Verma via GitGitGadget","Jeff King","Carlo Arenas","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":1},"messages":[{"id":"386937","messageId":"177deddcf83c2550c0db536a7a6942ba69a92fa5.1574600963.git.gitgitgadget@gmail.com","threadId":"52326","inReplyTo":"pull.473.git.1574600963.gitgitgadget@gmail.com","subject":"[PATCH 1/1] git-compat-util.h: drop the `PRIuMAX` definition","fromName":"Hariom Verma via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-11-24T13:09:23Z","receivedAt":"2019-11-24T13:09:35Z","isPatch":true,"sender":{"key":"name:Hariom Verma","avatar":null},"body":"From: Hariom Verma \n\nGit's code base already seems to be using `PRIdMAX` without any such\nfallback definition for quite a while (75459410edd (json_writer: new\nroutines to create JSON data, 2018-07-13), to be precise, and the\nfirst Git version to include that commit was v2.19.0).\n\nTherefore it should be safe to drop the fallback definition for\n`PRIuMAX` in `git-compat-util.h`.\n\nThis addresses https://github.com/gitgitgadget/git/issues/399\n\nSigned-off-by: Hariom Verma \n---\n git-compat-util.h | 4 ----\n 1 file changed, 4 deletions(-)\n\ndiff --git a/git-compat-util.h b/git-compat-util.h\nindex 607dca7534..ba710cfa6c 100644\n--- a/git-compat-util.h\n+++ b/git-compat-util.h\n@@ -320,10 +320,6 @@ char *gitdirname(char *);\n #define PATH_MAX 4096\n #endif\n \n-#ifndef PRIuMAX\n-#define PRIuMAX \"llu\"\n-#endif\n-\n #ifndef SCNuMAX\n #define SCNuMAX PRIuMAX\n #endif\n-- \ngitgitgadget\n"},{"id":"386938","messageId":"pull.473.git.1574600963.gitgitgadget@gmail.com","threadId":"52326","inReplyTo":null,"subject":"[PATCH 0/1] git-compat-util.h: drop the PRIuMAX definition","fromName":"Hariom Verma via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-11-24T13:09:22Z","receivedAt":"2019-11-24T13:09:35Z","isPatch":true,"sender":{"key":"name:Hariom Verma","avatar":null},"body":"Git's code base already seems to be using PRIdMAX without any such fallback\ndefinition for quite a while (75459410edd (json_writer: new routines to\ncreate JSON data, 2018-07-13), to be precise, and the first Git version to\ninclude that commit was v2.19.0).\n\nTherefore it should be safe to drop the fallback definition for PRIuMAX in\ngit-compat-util.h.\n\nThis addresses https://github.com/gitgitgadget/git/issues/399\n\nHariom Verma (1):\n git-compat-util.h: drop the `PRIuMAX` definition\n\n git-compat-util.h | 4 ----\n 1 file changed, 4 deletions(-)\n\n\nbase-commit: d9f6f3b6195a0ca35642561e530798ad1469bd41\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-473%2Fharry-hov%2Fpriumax-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-473/harry-hov/priumax-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/473\n-- \ngitgitgadget\n"},{"id":"386941","messageId":"20191124170643.GA16907@sigill.intra.peff.net","threadId":"52326","inReplyTo":"177deddcf83c2550c0db536a7a6942ba69a92fa5.1574600963.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 1/1] git-compat-util.h: drop the `PRIuMAX` definition","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-11-24T17:06:43Z","receivedAt":"2019-11-24T17:06:46Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Nov 24, 2019 at 01:09:23PM +0000, Hariom Verma via GitGitGadget wrote:\n\n> From: Hariom Verma \n> \n> Git's code base already seems to be using `PRIdMAX` without any such\n> fallback definition for quite a while (75459410edd (json_writer: new\n> routines to create JSON data, 2018-07-13), to be precise, and the\n> first Git version to include that commit was v2.19.0).\n> \n> Therefore it should be safe to drop the fallback definition for\n> `PRIuMAX` in `git-compat-util.h`.\n\nI noticed this recently, too, and wondered if it was time for a cleanup.\n\nWe do sometimes get portability reports more than a year after the\nproblem was introduced. But I think this one is pretty safe. PRIuMAX is\nin C99, and we've been picking up other C99-isms without complaint.\n\nI was curious what system originally spurred this. The PRIuMAX\ndefinition was originally added in 3efb1f343a (Check for PRIuMAX rather\nthan NO_C99_FORMAT in fast-import.c., 2007-02-20). But it was replacing\na construct that was introduced in 579d1fbfaf (Add NO_C99_FORMAT to\nsupport older compilers., 2006-07-30), which talks about gcc 2.95.\nThat's pretty ancient at this point.\n\n> diff --git a/git-compat-util.h b/git-compat-util.h\n> index 607dca7534..ba710cfa6c 100644\n> --- a/git-compat-util.h\n> +++ b/git-compat-util.h\n> @@ -320,10 +320,6 @@ char *gitdirname(char *);\n> #define PATH_MAX 4096\n> #endif\n> \n> -#ifndef PRIuMAX\n> -#define PRIuMAX \"llu\"\n> -#endif\n> -\n\nThis part of the patch looks obviously correct. :) But...\n\n> #ifndef SCNuMAX\n> #define SCNuMAX PRIuMAX\n> #endif\n\nCan we likewise ditch the fallback definition for SCNuMAX? And PRIu32,\netc? It seems likely any platform would either have all of them or none.\n\n-Peff\n"},{"id":"386942","messageId":"CAPUEspjAjR+QojRbC8dkoj=9adAKEv4StYYiy6hFmbMwQZ7CNw@mail.gmail.com","threadId":"52326","inReplyTo":"20191124170643.GA16907@sigill.intra.peff.net","subject":"Re: [PATCH 1/1] git-compat-util.h: drop the `PRIuMAX` definition","fromName":"Carlo Arenas","fromEmail":"carenas@gmail.com","sentAt":"2019-11-24T17:40:15Z","receivedAt":"2019-11-24T17:40:31Z","isPatch":true,"sender":{"key":"carenas@gmail.com","avatar":"https://avatars.githubusercontent.com/u/76036?v=4"},"body":"On Sun, Nov 24, 2019 at 9:11 AM Jeff King wrote:\n>\n> We do sometimes get portability reports more than a year after the\n> problem was introduced. But I think this one is pretty safe. PRIuMAX is\n> in C99, and we've been picking up other C99-isms without complaint.\n\nI think the problem might come from places where the default compiler\nis still not C99 by default (ex: old CentOS) and any other places\nusing gcc < 5 which is less ancient\n\nCarlo\n"},{"id":"386950","messageId":"CAPUEspjTpWsaT=pm1HLwTUQQye+WMCeWwpmC9RWkBuKQELubRw@mail.gmail.com","threadId":"52326","inReplyTo":"CAPUEspjAjR+QojRbC8dkoj=9adAKEv4StYYiy6hFmbMwQZ7CNw@mail.gmail.com","subject":"Re: [PATCH 1/1] git-compat-util.h: drop the `PRIuMAX` definition","fromName":"Carlo Arenas","fromEmail":"carenas@gmail.com","sentAt":"2019-11-24T20:15:47Z","receivedAt":"2019-11-24T20:16:17Z","isPatch":true,"sender":{"key":"carenas@gmail.com","avatar":"https://avatars.githubusercontent.com/u/76036?v=4"},"body":"On Sun, Nov 24, 2019 at 9:40 AM Carlo Arenas wrote:\n>\n> I think the problem might come from places where the default compiler\n> is still not C99 by default (ex: old CentOS)\n\nCentOS 6.10 (using gcc 4.4.7) compiles this without problems by\ndefault so nothing to worry about, sorry for the red herring\n\nCarlo\n\nPS. non GNU89 (AKA ANSI) mode will obviously break but not ONLY\nbecause of this change\n"},{"id":"386964","messageId":"xmqq1rtwzoal.fsf@gitster-ct.c.googlers.com","threadId":"52326","inReplyTo":"20191124170643.GA16907@sigill.intra.peff.net","subject":"Re: [PATCH 1/1] git-compat-util.h: drop the `PRIuMAX` definition","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-11-25T02:24:02Z","receivedAt":"2019-11-25T02:24:14Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Sun, Nov 24, 2019 at 01:09:23PM +0000, Hariom Verma via GitGitGadget wrote:\n>\n>> From: Hariom Verma \n>> \n>> Git's code base already seems to be using `PRIdMAX` without any such\n>> fallback definition for quite a while (75459410edd (json_writer: new\n>> routines to create JSON data, 2018-07-13), to be precise, and the\n>> first Git version to include that commit was v2.19.0).\n>> \n>> Therefore it should be safe to drop the fallback definition for\n>> `PRIuMAX` in `git-compat-util.h`.\n>\n> I noticed this recently, too, and wondered if it was time for a cleanup.\n\nWhile I agree with the conclusion, I do not think I agree with the\nabove \"Therefore (implying that the lack of need for fallback\nPRIdMAX means the same for PRIuMAX) it should be safe\" as a good\njustification. That reasoning assumes that the outside world is\nmuch saner than us. We thought PRIuMAX fallback necessary while a\ncounterpart for PRIdMAX unneeded---the outside world could have made\na similar mistake and in the opposite way (i.e. only defined PRIdMAX\nwhile leaving PRIuMAX undefined).\n\nBut I do agree with the alternative justification in the following\ntwo paragraphs you have given, which are ...\n\n> We do sometimes get portability reports more than a year after the\n> problem was introduced. But I think this one is pretty safe. PRIuMAX is\n> in C99, and we've been picking up other C99-isms without complaint.\n>\n> I was curious what system originally spurred this. The PRIuMAX\n> definition was originally added in 3efb1f343a (Check for PRIuMAX rather\n> than NO_C99_FORMAT in fast-import.c., 2007-02-20). But it was replacing\n> a construct that was introduced in 579d1fbfaf (Add NO_C99_FORMAT to\n> support older compilers., 2006-07-30), which talks about gcc 2.95.\n> That's pretty ancient at this point.\n\n... these.\n\n> This part of the patch looks obviously correct. :) But...\n>\n>> #ifndef SCNuMAX\n>> #define SCNuMAX PRIuMAX\n>> #endif\n>\n> Can we likewise ditch the fallback definition for SCNuMAX? And PRIu32,\n> etc? It seems likely any platform would either have all of them or none.\n\nI guess that's also a C99-ism that we can use?\n\nThanks, both.\n"},{"id":"386965","messageId":"xmqqv9r8y8qe.fsf@gitster-ct.c.googlers.com","threadId":"52326","inReplyTo":"xmqq1rtwzoal.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH 1/1] git-compat-util.h: drop the `PRIuMAX` definition","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-11-25T02:45:29Z","receivedAt":"2019-11-25T02:45:38Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n>> Can we likewise ditch the fallback definition for SCNuMAX? And PRIu32,\n>> etc? It seems likely any platform would either have all of them or none.\n>\n> I guess that's also a C99-ism that we can use?\n>\n> Thanks, both.\n\nHere is what I have locally for now.\n\n1: 98f866a929 ! 1: ebc3278665 git-compat-util.h: drop the `PRIuMAX` definition\n @@ Metadata\n Author: Hariom Verma \n \n ## Commit message ##\n - git-compat-util.h: drop the `PRIuMAX` definition\n + git-compat-util.h: drop the `PRIuMAX` and other fallback definitions\n \n Git's code base already seems to be using `PRIdMAX` without any such\n fallback definition for quite a while (75459410edd (json_writer: new\n routines to create JSON data, 2018-07-13), to be precise, and the\n - first Git version to include that commit was v2.19.0).\n + first Git version to include that commit was v2.19.0). Having a\n + fallback definition only for `PRIuMAX` is a bit inconsistent.\n \n - Therefore it should be safe to drop the fallback definition for\n - `PRIuMAX` in `git-compat-util.h`.\n + We do sometimes get portability reports more than a year after the\n + problem was introduced. This one should be fairly safe. PRIuMAX is\n + in C99 (for that matter, SCNuMAX, PRIu32 and others also are), and\n + we've been picking up other C99-isms without complaint.\n \n - This addresses https://github.com/gitgitgadget/git/issues/399\n + The PRIuMAX fallback definition was originally added in 3efb1f343a\n + (Check for PRIuMAX rather than NO_C99_FORMAT in fast-import.c.,\n + 2007-02-20). But it was replacing a construct that was introduced in\n + an even earlier commit, 579d1fbfaf (Add NO_C99_FORMAT to support\n + older compilers., 2006-07-30), which talks about gcc 2.95.\n +\n + That's pretty ancient at this point.\n \n Signed-off-by: Hariom Verma \n + Helped-by: Jeff King \n + [jc: tweaked both message and code, taking what peff wrote]\n Signed-off-by: Junio C Hamano \n \n ## git-compat-util.h ##\n @@ git-compat-util.h: char *gitdirname(char *);\n -#define PRIuMAX \"llu\"\n -#endif\n -\n - #ifndef SCNuMAX\n - #define SCNuMAX PRIuMAX\n - #endif\n +-#ifndef SCNuMAX\n +-#define SCNuMAX PRIuMAX\n +-#endif\n +-\n +-#ifndef PRIu32\n +-#define PRIu32 \"u\"\n +-#endif\n +-\n +-#ifndef PRIx32\n +-#define PRIx32 \"x\"\n +-#endif\n +-\n +-#ifndef PRIo32\n +-#define PRIo32 \"o\"\n +-#endif\n +-\n + typedef uintmax_t timestamp_t;\n + #define PRItime PRIuMAX\n + #define parse_timestamp strtoumax\n"},{"id":"386979","messageId":"20191125093400.GA25851@sigill.intra.peff.net","threadId":"52326","inReplyTo":"xmqqv9r8y8qe.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH 1/1] git-compat-util.h: drop the `PRIuMAX` definition","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-11-25T09:34:00Z","receivedAt":"2019-11-25T09:34:03Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Nov 25, 2019 at 11:45:29AM +0900, Junio C Hamano wrote:\n\n> Junio C Hamano writes:\n> \n> >> Can we likewise ditch the fallback definition for SCNuMAX? And PRIu32,\n> >> etc? It seems likely any platform would either have all of them or none.\n> >\n> > I guess that's also a C99-ism that we can use?\n> >\n> > Thanks, both.\n> \n> Here is what I have locally for now.\n\nYes, that looks good to me (and yes, those other format macros are in\nC99, too).\n\n-Peff\n"}]}