{"thread":{"id":"63575","subject":"[PATCH 4/4] Fix unreachable-code warning with clang on Windows","startedAt":"2025-06-03T23:07:35Z","lastAt":"2025-06-06T00:23:31Z","messageCount":11,"participants":["Mike Hommey","Junio C Hamano","Patrick Steinhardt","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":4},"messages":[{"id":"519613","messageId":"20250603230646.2322671-4-mh@glandium.org","threadId":"63575","inReplyTo":"20250603230646.2322671-1-mh@glandium.org","subject":"[PATCH 4/4] Fix unreachable-code warning with clang on Windows","fromName":"Mike Hommey","fromEmail":"mh@glandium.org","sentAt":"2025-06-03T23:06:46Z","receivedAt":"2025-06-03T23:07:35Z","isPatch":true,"sender":{"key":"mh@glandium.org","avatar":"https://avatars.githubusercontent.com/u/1038527?v=4"},"body":"```\nrefs/files-backend.c:3187:5: error: code will never be executed [-Werror,-Wunreachable-code]\n 3187 | continue;\n | ^~~~~~~~\n 1 error generated.\n```\n\nSigned-off-by: Mike Hommey \n---\n refs/files-backend.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/refs/files-backend.c b/refs/files-backend.c\nindex bf6f89b1d1..af21eb80a9 100644\n--- a/refs/files-backend.c\n+++ b/refs/files-backend.c\n@@ -3183,7 +3183,7 @@ static int files_transaction_finish(struct ref_store *ref_store,\n \t\t * next update. If not, we try and create a regular symref.\n \t\t */\n \t\tif (update->new_target && refs->prefer_symlink_refs)\n-\t\t\tif (!create_ref_symlink(lock, update->new_target))\n+\t\t\tif (NOT_CONSTANT(!create_ref_symlink(lock, update->new_target)))\n \t\t\t\tcontinue;\n \n \t\tif (update->flags & REF_NEEDS_COMMIT) {\n-- \n2.50.0.rc1.593.g042f21cb9b\n\n"},{"id":"519614","messageId":"20250603230646.2322671-3-mh@glandium.org","threadId":"63575","inReplyTo":"20250603230646.2322671-1-mh@glandium.org","subject":"[PATCH 3/4] Fix comma warnings with clang on Windows","fromName":"Mike Hommey","fromEmail":"mh@glandium.org","sentAt":"2025-06-03T23:06:45Z","receivedAt":"2025-06-03T23:07:35Z","isPatch":true,"sender":{"key":"mh@glandium.org","avatar":"https://avatars.githubusercontent.com/u/1038527?v=4"},"body":"e.g.\n```\n compat/mingw.c:723:24: error: possible misuse of comma operator here [-Werror,-Wcomma]\n 723 | return errno = ENOSYS, -1;\n | ^\n compat/mingw.c:723:10: note: cast expression to void to silence warning\n 723 | return errno = ENOSYS, -1;\n | ^~~~~~~~~~~~~~\n | (void)( )\n /usr/x86_64-w64-mingw32/include/stdlib.h:155:15: note: expanded from macro 'errno'\n 155 | #define errno (*_errno())\n | ^\n```\n\nSigned-off-by: Mike Hommey \n---\n compat/mingw.c | 48 ++++++++++++++++++++++++++++--------------------\n 1 file changed, 28 insertions(+), 20 deletions(-)\n\ndiff --git a/compat/mingw.c b/compat/mingw.c\nindex 8a9972a1ca..cd54937ebd 100644\n--- a/compat/mingw.c\n+++ b/compat/mingw.c\n@@ -501,8 +501,10 @@ static int mingw_open_append(wchar_t const *wfilename, int oflags, ...)\n \tDWORD create = (oflags & O_CREAT) ? OPEN_ALWAYS : OPEN_EXISTING;\n \n \t/* only these flags are supported */\n-\tif ((oflags & ~O_CREAT) != (O_WRONLY | O_APPEND))\n-\t\treturn errno = ENOSYS, -1;\n+\tif ((oflags & ~O_CREAT) != (O_WRONLY | O_APPEND)) {\n+\t\terrno = ENOSYS;\n+\t\treturn -1;\n+\t}\n \n \t/*\n \t * FILE_SHARE_WRITE is required to permit child processes\n@@ -2497,12 +2499,14 @@ static int start_timer_thread(void)\n \ttimer_event = CreateEvent(NULL, FALSE, FALSE, NULL);\n \tif (timer_event) {\n \t\ttimer_thread = (HANDLE) _beginthreadex(NULL, 0, ticktack, NULL, 0, NULL);\n-\t\tif (!timer_thread )\n-\t\t\treturn errno = ENOMEM,\n-\t\t\t\terror(\"cannot start timer thread\");\n-\t} else\n-\t\treturn errno = ENOMEM,\n-\t\t\terror(\"cannot allocate resources for timer\");\n+\t\tif (!timer_thread) {\n+\t\t\terrno = ENOMEM;\n+\t\t\treturn error(\"cannot start timer thread\");\n+\t\t}\n+\t} else {\n+\t\terrno = ENOMEM;\n+\t\treturn error(\"cannot allocate resources for timer\");\n+\t}\n \treturn 0;\n }\n \n@@ -2535,13 +2539,15 @@ int setitimer(int type UNUSED, struct itimerval *in, struct itimerval *out)\n \tstatic const struct timeval zero;\n \tstatic int atexit_done;\n \n-\tif (out)\n-\t\treturn errno = EINVAL,\n-\t\t\terror(\"setitimer param 3 != NULL not implemented\");\n+\tif (out) {\n+\t\terrno = EINVAL;\n+\t\treturn error(\"setitimer param 3 != NULL not implemented\");\n+\t}\n \tif (!is_timeval_eq(&in->it_interval, &zero) &&\n-\t !is_timeval_eq(&in->it_interval, &in->it_value))\n-\t\treturn errno = EINVAL,\n-\t\t\terror(\"setitimer: it_interval must be zero or eq it_value\");\n+\t !is_timeval_eq(&in->it_interval, &in->it_value)) {\n+\t\terrno = EINVAL;\n+\t\treturn error(\"setitimer: it_interval must be zero or eq it_value\");\n+\t}\n \n \tif (timer_thread)\n \t\tstop_timer_thread();\n@@ -2561,12 +2567,14 @@ int setitimer(int type UNUSED, struct itimerval *in, struct itimerval *out)\n \n int sigaction(int sig, struct sigaction *in, struct sigaction *out)\n {\n-\tif (sig != SIGALRM)\n-\t\treturn errno = EINVAL,\n-\t\t\terror(\"sigaction only implemented for SIGALRM\");\n-\tif (out)\n-\t\treturn errno = EINVAL,\n-\t\t\terror(\"sigaction: param 3 != NULL not implemented\");\n+\tif (sig != SIGALRM) {\n+\t\terrno = EINVAL;\n+\t\treturn error(\"sigaction only implemented for SIGALRM\");\n+\t}\n+\tif (out) {\n+\t\terrno = EINVAL;\n+\t\treturn error(\"sigaction: param 3 != NULL not implemented\");\n+\t}\n \n \ttimer_fn = in->sa_handler;\n \treturn 0;\n-- \n2.50.0.rc1.593.g042f21cb9b\n\n"},{"id":"519615","messageId":"20250603230646.2322671-1-mh@glandium.org","threadId":"63575","inReplyTo":null,"subject":"[PATCH 1/4] Fix maybe-uninitialized warning with GCC at -O3","fromName":"Mike Hommey","fromEmail":"mh@glandium.org","sentAt":"2025-06-03T23:06:43Z","receivedAt":"2025-06-03T23:07:35Z","isPatch":true,"sender":{"key":"mh@glandium.org","avatar":"https://avatars.githubusercontent.com/u/1038527?v=4"},"body":"```\nIn file included from parse-options.c:1:\ngit-compat-util.h: In function ‘get_value’:\ngit-compat-util.h:489:21: error: ‘arg’ may be used uninitialized [-Werror=maybe-uninitialized]\n 489 | #define error(...) (error(__VA_ARGS__), const_error())\n | ^~~~~\nparse-options.c:76:21: note: ‘arg’ was declared here\n 76 | const char *arg;\n | ^~~\n```\n\nSigned-off-by: Mike Hommey \n---\n parse-options.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/parse-options.c b/parse-options.c\nindex a9a39ecaef..cf79805bc0 100644\n--- a/parse-options.c\n+++ b/parse-options.c\n@@ -73,7 +73,7 @@ static enum parse_opt_result do_get_value(struct parse_opt_ctx_t *p,\n \t\t\t\t\t enum opt_parsed flags,\n \t\t\t\t\t const char **argp)\n {\n-\tconst char *arg;\n+\tconst char *arg = NULL;\n \tconst int unset = flags & OPT_UNSET;\n \tint err;\n \n-- \n2.50.0.rc1.593.g042f21cb9b\n\n"},{"id":"519616","messageId":"20250603230646.2322671-2-mh@glandium.org","threadId":"63575","inReplyTo":"20250603230646.2322671-1-mh@glandium.org","subject":"[PATCH 2/4] Fix use-after-free warning with GCC at -O3","fromName":"Mike Hommey","fromEmail":"mh@glandium.org","sentAt":"2025-06-03T23:06:44Z","receivedAt":"2025-06-03T23:07:35Z","isPatch":true,"sender":{"key":"mh@glandium.org","avatar":"https://avatars.githubusercontent.com/u/1038527?v=4"},"body":"```\nreftable/basics.c: In function ‘parse_names’:\nreftable/basics.c:233:17: error: pointer ‘names’ may be used after ‘free’ [-Werror=use-after-free]\n 233 | reftable_free(names[i]);\n | ^~~~~~~~~~~~~~~~~~~~~~~\nIn function ‘reftable_free’,\n inlined from ‘reftable_realloc’ at reftable/basics.c:30:3,\n inlined from ‘reftable_realloc’ at reftable/basics.c:27:7,\n inlined from ‘reftable_alloc_grow’ at reftable/basics.h:228:10,\n inlined from ‘parse_names’ at reftable/basics.c:214:8:\nreftable/basics.c:44:17: note: call to ‘free’ here\n 44 | free(p);\n | ^~~~~~~\n```\n\nSigned-off-by: Mike Hommey \n---\n reftable/basics.c | 8 +++++---\n 1 file changed, 5 insertions(+), 3 deletions(-)\n\ndiff --git a/reftable/basics.c b/reftable/basics.c\nindex 9988ebd635..de21fe6ef7 100644\n--- a/reftable/basics.c\n+++ b/reftable/basics.c\n@@ -229,9 +229,11 @@ char **parse_names(char *buf, int size)\n \treturn names;\n \n err:\n-\tfor (size_t i = 0; i < names_len; i++)\n-\t\treftable_free(names[i]);\n-\treftable_free(names);\n+\tif (names) {\n+\t\tfor (size_t i = 0; i < names_len; i++)\n+\t\t\treftable_free(names[i]);\n+\t\treftable_free(names);\n+\t}\n \treturn NULL;\n }\n \n-- \n2.50.0.rc1.593.g042f21cb9b\n\n"},{"id":"519619","messageId":"xmqqwm9sfkuu.fsf@gitster.g","threadId":"63575","inReplyTo":"20250603230646.2322671-2-mh@glandium.org","subject":"Re: [PATCH 2/4] Fix use-after-free warning with GCC at -O3","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-06-03T23:47:53Z","receivedAt":"2025-06-03T23:47:56Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Mike Hommey writes:\n\n> Cc: gitster@pobox.com, Mike Hommey \n\nNot to me, but to the designated area expert.\n\nThanks.\n\n> ```\n> reftable/basics.c: In function ‘parse_names’:\n> reftable/basics.c:233:17: error: pointer ‘names’ may be used after ‘free’ [-Werror=use-after-free]\n> 233 | reftable_free(names[i]);\n> | ^~~~~~~~~~~~~~~~~~~~~~~\n> In function ‘reftable_free’,\n> inlined from ‘reftable_realloc’ at reftable/basics.c:30:3,\n> inlined from ‘reftable_realloc’ at reftable/basics.c:27:7,\n> inlined from ‘reftable_alloc_grow’ at reftable/basics.h:228:10,\n> inlined from ‘parse_names’ at reftable/basics.c:214:8:\n> reftable/basics.c:44:17: note: call to ‘free’ here\n> 44 | free(p);\n> | ^~~~~~~\n> ```\n>\n> Signed-off-by: Mike Hommey \n> ---\n> reftable/basics.c | 8 +++++---\n> 1 file changed, 5 insertions(+), 3 deletions(-)\n>\n> diff --git a/reftable/basics.c b/reftable/basics.c\n> index 9988ebd635..de21fe6ef7 100644\n> --- a/reftable/basics.c\n> +++ b/reftable/basics.c\n> @@ -229,9 +229,11 @@ char **parse_names(char *buf, int size)\n> \treturn names;\n> \n> err:\n> -\tfor (size_t i = 0; i < names_len; i++)\n> -\t\treftable_free(names[i]);\n> -\treftable_free(names);\n> +\tif (names) {\n> +\t\tfor (size_t i = 0; i < names_len; i++)\n> +\t\t\treftable_free(names[i]);\n> +\t\treftable_free(names);\n> +\t}\n> \treturn NULL;\n> }\n"},{"id":"519620","messageId":"xmqqplfkfkol.fsf@gitster.g","threadId":"63575","inReplyTo":"20250603230646.2322671-3-mh@glandium.org","subject":"Re: [PATCH 3/4] Fix comma warnings with clang on Windows","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-06-03T23:51:38Z","receivedAt":"2025-06-03T23:51:40Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Mike Hommey writes:\n\n> Subject: Re: [PATCH 3/4] Fix comma warnings with clang on Windows\n\nCommon to all four patches, as \"Fix\" does not quite tell the story,\nplease choose more appropriate verb. For example, judging from how\nthis was rewritten ...\n\n> -\tif ((oflags & ~O_CREAT) != (O_WRONLY | O_APPEND))\n> -\t\treturn errno = ENOSYS, -1;\n> +\tif ((oflags & ~O_CREAT) != (O_WRONLY | O_APPEND)) {\n> +\t\terrno = ENOSYS;\n> +\t\treturn -1;\n> +\t}\n\n... the warning is a false positive (i.e. the code does what the\nauthor intended it to do), so this is more like \"squelching\" the\nwarning.\n\nI obviously like the style after this patch. I just thought that\nthe proposed log message, especially its title, were not clear\nenough to tell which ones are real fixes and which ones are\nworkarounds.\n\nThanks.\n"},{"id":"519633","messageId":"aD_3Y0PQtfg8Dd9z@pks.im","threadId":"63575","inReplyTo":"20250603230646.2322671-1-mh@glandium.org","subject":"Re: [PATCH 1/4] Fix maybe-uninitialized warning with GCC at -O3","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-06-04T07:36:03Z","receivedAt":"2025-06-04T07:36:07Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Wed, Jun 04, 2025 at 08:06:43AM +0900, Mike Hommey wrote:\n> ```\n> In file included from parse-options.c:1:\n> git-compat-util.h: In function ‘get_value’:\n> git-compat-util.h:489:21: error: ‘arg’ may be used uninitialized [-Werror=maybe-uninitialized]\n> 489 | #define error(...) (error(__VA_ARGS__), const_error())\n> | ^~~~~\n> parse-options.c:76:21: note: ‘arg’ was declared here\n> 76 | const char *arg;\n> | ^~~\n> ```\n\nA bit more explanation whether this warning is a false positive or\nwhether this may be an actual issue would be welcome.\n\nPatrick\n"},{"id":"519634","messageId":"aD_3ZzWSbyXIk81_@pks.im","threadId":"63575","inReplyTo":"20250603230646.2322671-2-mh@glandium.org","subject":"Re: [PATCH 2/4] Fix use-after-free warning with GCC at -O3","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-06-04T07:36:07Z","receivedAt":"2025-06-04T07:36:10Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Wed, Jun 04, 2025 at 08:06:44AM +0900, Mike Hommey wrote:\n> ```\n> reftable/basics.c: In function ‘parse_names’:\n> reftable/basics.c:233:17: error: pointer ‘names’ may be used after ‘free’ [-Werror=use-after-free]\n> 233 | reftable_free(names[i]);\n> | ^~~~~~~~~~~~~~~~~~~~~~~\n> In function ‘reftable_free’,\n> inlined from ‘reftable_realloc’ at reftable/basics.c:30:3,\n> inlined from ‘reftable_realloc’ at reftable/basics.c:27:7,\n> inlined from ‘reftable_alloc_grow’ at reftable/basics.h:228:10,\n> inlined from ‘parse_names’ at reftable/basics.c:214:8:\n> reftable/basics.c:44:17: note: call to ‘free’ here\n> 44 | free(p);\n> | ^~~~~~~\n> ```\n\nSame here, only posting the warning isn't sufficient to explain what's\ngoing on.\n\n> diff --git a/reftable/basics.c b/reftable/basics.c\n> index 9988ebd635..de21fe6ef7 100644\n> --- a/reftable/basics.c\n> +++ b/reftable/basics.c\n> @@ -229,9 +229,11 @@ char **parse_names(char *buf, int size)\n> \treturn names;\n> \n> err:\n> -\tfor (size_t i = 0; i < names_len; i++)\n> -\t\treftable_free(names[i]);\n> -\treftable_free(names);\n> +\tif (names) {\n> +\t\tfor (size_t i = 0; i < names_len; i++)\n> +\t\t\treftable_free(names[i]);\n> +\t\treftable_free(names);\n> +\t}\n> \treturn NULL;\n> }\n\nThis change shouldn't be needed in theory: `names_len` has a positive\nvalue if and only if `names` is non-NULL. So the warning is a false\npositive.\n\nThat being said I'm not opposed to squelching this warning. But details\nlike this should be explained in the commit message.\n\nPatrick\n"},{"id":"519635","messageId":"aD_3ahX2jyrtfvjq@pks.im","threadId":"63575","inReplyTo":"20250603230646.2322671-4-mh@glandium.org","subject":"Re: [PATCH 4/4] Fix unreachable-code warning with clang on Windows","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-06-04T07:36:10Z","receivedAt":"2025-06-04T07:36:13Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Wed, Jun 04, 2025 at 08:06:46AM +0900, Mike Hommey wrote:\n> ```\n> refs/files-backend.c:3187:5: error: code will never be executed [-Werror,-Wunreachable-code]\n> 3187 | continue;\n> | ^~~~~~~~\n> 1 error generated.\n> ```\n> \n> Signed-off-by: Mike Hommey \n> ---\n> refs/files-backend.c | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n> \n> diff --git a/refs/files-backend.c b/refs/files-backend.c\n> index bf6f89b1d1..af21eb80a9 100644\n> --- a/refs/files-backend.c\n> +++ b/refs/files-backend.c\n> @@ -3183,7 +3183,7 @@ static int files_transaction_finish(struct ref_store *ref_store,\n> \t\t * next update. If not, we try and create a regular symref.\n> \t\t */\n> \t\tif (update->new_target && refs->prefer_symlink_refs)\n> -\t\t\tif (!create_ref_symlink(lock, update->new_target))\n> +\t\t\tif (NOT_CONSTANT(!create_ref_symlink(lock, update->new_target)))\n> \t\t\t\tcontinue;\n\nSo the story here is that there are two implementations of\n`create_ref_symlink()`:\n\n - One macro that is defined to `(-1)` which is set when\n NO_SYMLINK_HEAD is defined.\n\n - A function that creates the ref symlink if NO_SYMLINK_HEAD is not\n defined.\n\nThe function won't cause the error, but the macro will. So wouldn't it\nmake more sense to wrap the macro itself in `NOT_CONSTANT`, like this:\n\n #define create_ref_symlink(a, b) NOT_CONSTANT(-1)\n\nPatrick\n"},{"id":"519678","messageId":"xmqq8qm7cxq3.fsf@gitster.g","threadId":"63575","inReplyTo":"aD_3ahX2jyrtfvjq@pks.im","subject":"Re: [PATCH 4/4] Fix unreachable-code warning with clang on Windows","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-06-04T15:50:28Z","receivedAt":"2025-06-04T15:50:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Patrick Steinhardt writes:\n\n> The function won't cause the error, but the macro will. So wouldn't it\n> make more sense to wrap the macro itself in `NOT_CONSTANT`, like this:\n>\n> #define create_ref_symlink(a, b) NOT_CONSTANT(-1)\n\nThat's clever ;-).\n\n\nWe cannot unfortunatel do the same at the site that the macro\nNOT_CONSTANT() was invented for, though.\n\n\n"},{"id":"519816","messageId":"20250606002329.GA3556939@coredump.intra.peff.net","threadId":"63575","inReplyTo":"aD_3Y0PQtfg8Dd9z@pks.im","subject":"Re: [PATCH 1/4] Fix maybe-uninitialized warning with GCC at -O3","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2025-06-06T00:23:29Z","receivedAt":"2025-06-06T00:23:31Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Jun 04, 2025 at 09:36:03AM +0200, Patrick Steinhardt wrote:\n\n> On Wed, Jun 04, 2025 at 08:06:43AM +0900, Mike Hommey wrote:\n> > ```\n> > In file included from parse-options.c:1:\n> > git-compat-util.h: In function ‘get_value’:\n> > git-compat-util.h:489:21: error: ‘arg’ may be used uninitialized [-Werror=maybe-uninitialized]\n> > 489 | #define error(...) (error(__VA_ARGS__), const_error())\n> > | ^~~~~\n> > parse-options.c:76:21: note: ‘arg’ was declared here\n> > 76 | const char *arg;\n> > | ^~~\n> > ```\n> \n> A bit more explanation whether this warning is a false positive or\n> whether this may be an actual issue would be welcome.\n\nI thought at first it was a false positive, as we'd always fill in \"arg\"\nvia get_arg(), or return an error. But I suspect the culprit is the\nearlier part of this if/else chain:\n\n\n if (unset) {\n value = 0;\n } else if (opt->flags & PARSE_OPT_OPTARG && !p->opt) {\n value = opt->defval;\n } else if (get_arg(p, opt, flags, &arg)) {\n return -1;\n\t\t } ...\n\nSo if \"unset\" is true, we set \"value\" but not \"arg\". The code the\ncompiler is complaining about is here:\n\n if (value < lower_bound)\n return error(_(\"value %s for %s not in range [%\"PRIdMAX\",%\"PRIdMAX\"]\"),\n arg, optname(opt, flags), (intmax_t)lower_bound, (intmax_t)upper_bound);\n\nIn the case of \"unset\", I think we could never trigger this, since our\nlower bound will never be above 0.\n\nBut what about opt->defval? We don't know anything about it here.\nProbably it would be a programming error to pass in a value that is\noutside the bounds, but this code doesn't know that. So something like\nthis (it was actually hard to find an integer option with OPTARG!):\n\ndiff --git a/builtin/fmt-merge-msg.c b/builtin/fmt-merge-msg.c\nindex 3b6aac2cf7..630403611b 100644\n--- a/builtin/fmt-merge-msg.c\n+++ b/builtin/fmt-merge-msg.c\n@@ -28,7 +28,7 @@ int cmd_fmt_merge_msg(int argc,\n \t\t\t.argh = N_(\"n\"),\n \t\t\t.help = N_(\"populate log with at most entries from shortlog\"),\n \t\t\t.flags = PARSE_OPT_OPTARG,\n-\t\t\t.defval = DEFAULT_MERGE_LOG_LEN,\n+\t\t\t.defval = -2147483649,\n \t\t},\n \t\t{\n \t\t\t.type = OPTION_INTEGER,\n\nwould cause:\n\n git fmt-merge-msg --log\n\nto print uninitialized memory. But it is equally wrong with Mike's\npatch; we'd just segfault!\n\nI think this is unlikely to happen in practice, but it does feel like we\nshould be able to solve it in a better way. I came up with two options.\n\nOne is to BUG() on a bogus default value, like this:\n\ndiff --git a/parse-options.c b/parse-options.c\nindex a9a39ecaef..7ddf213d0c 100644\n--- a/parse-options.c\n+++ b/parse-options.c\n@@ -73,7 +73,7 @@ static enum parse_opt_result do_get_value(struct parse_opt_ctx_t *p,\n \t\t\t\t\t enum opt_parsed flags,\n \t\t\t\t\t const char **argp)\n {\n-\tconst char *arg;\n+\tconst char *arg = NULL;\n \tconst int unset = flags & OPT_UNSET;\n \tint err;\n \n@@ -195,9 +195,13 @@ static enum parse_opt_result do_get_value(struct parse_opt_ctx_t *p,\n \t\t\t\t optname(opt, flags));\n \t\t}\n \n-\t\tif (value < lower_bound)\n+\t\tif (value < lower_bound) {\n+\t\t\tif (!arg)\n+\t\t\t\tBUG(\"default option value for '%s' is not in range\",\n+\t\t\t\t optname(opt, flags));\n \t\t\treturn error(_(\"value %s for %s not in range [%\"PRIdMAX\",%\"PRIdMAX\"]\"),\n \t\t\t\t arg, optname(opt, flags), (intmax_t)lower_bound, (intmax_t)upper_bound);\n+\t\t}\n \n \t\tswitch (opt->precision) {\n \t\tcase 1:\n\nThe other is to not bother checking the bounds on the default values at\nall, by pushing this bounds check into the if/else chain after we know\nwe have something to parse, like this (extended context to make it more\nobvious how this fits into the chain):\n\ndiff --git a/parse-options.c b/parse-options.c\nindex a9a39ecaef..d36e56da15 100644\n--- a/parse-options.c\n+++ b/parse-options.c\n@@ -179,39 +179,38 @@ static enum parse_opt_result do_get_value(struct parse_opt_ctx_t *p,\n \n \t\tif (unset) {\n \t\t\tvalue = 0;\n \t\t} else if (opt->flags & PARSE_OPT_OPTARG && !p->opt) {\n \t\t\tvalue = opt->defval;\n \t\t} else if (get_arg(p, opt, flags, &arg)) {\n \t\t\treturn -1;\n \t\t} else if (!*arg) {\n \t\t\treturn error(_(\"%s expects a numerical value\"),\n \t\t\t\t optname(opt, flags));\n \t\t} else if (!git_parse_signed(arg, &value, upper_bound)) {\n \t\t\tif (errno == ERANGE)\n \t\t\t\treturn error(_(\"value %s for %s not in range [%\"PRIdMAX\",%\"PRIdMAX\"]\"),\n \t\t\t\t\t arg, optname(opt, flags), lower_bound, upper_bound);\n \n \t\t\treturn error(_(\"%s expects an integer value with an optional k/m/g suffix\"),\n \t\t\t\t optname(opt, flags));\n-\t\t}\n-\n-\t\tif (value < lower_bound)\n+\t\t} else if (value < lower_bound) {\n \t\t\treturn error(_(\"value %s for %s not in range [%\"PRIdMAX\",%\"PRIdMAX\"]\"),\n \t\t\t\t arg, optname(opt, flags), (intmax_t)lower_bound, (intmax_t)upper_bound);\n+\t\t}\n \n \t\tswitch (opt->precision) {\n \t\tcase 1:\n \t\t\t*(int8_t *)opt->value = value;\n \t\t\treturn 0;\n \t\tcase 2:\n \t\t\t*(int16_t *)opt->value = value;\n \t\t\treturn 0;\n \t\tcase 4:\n \t\t\t*(int32_t *)opt->value = value;\n \t\t\treturn 0;\n \t\tcase 8:\n \t\t\t*(int64_t *)opt->value = value;\n \t\t\treturn 0;\n \t\tdefault:\n \t\t\tBUG(\"invalid precision for option %s\",\n \t\t\t optname(opt, flags));\n\n-Peff\n"}]}