From patchwork Wed Apr 16 10:02:10 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 14053612 Received: from fout-a3-smtp.messagingengine.com (fout-a3-smtp.messagingengine.com [103.168.172.146]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 84198229B1E for ; Wed, 16 Apr 2025 10:02:29 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=103.168.172.146 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1744797752; cv=none; b=pD/wYUvS3YOg2MNHSeThGOHoIX/q+pUVmJu2ailKXHqYZuL0VyARw8pEqFrm7sosZKr2pTLvgYeUHRXPnTQ6m63lExINMQp0d0CTFpptJhd691MovNZAxE8grFbpPMBZlojyFn9RnsN7rgLMAbjJ5LHc+felbprm92OeqOlpxvY= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1744797752; c=relaxed/simple; bh=g28ml5GS+1JG0lkQQ40hkk9QkX5CAfm7BWn+bWfsKyw=; h=From:Date:Subject:MIME-Version:Content-Type:Message-Id:References: In-Reply-To:To:Cc; b=vF5tNITX6V9jImllUwZyyjmPjPFsXtNhE8MJFULpmG2dStRGpwDCPIuLeOi18zQ+6OOR4HrVAd9aNYGw4BzkE2rJ86hyiGshWtEE1n99H9RotaQqmqHpPFXjPMfz202RJwITwGniebdHG7geFlQSc5u8vmBte204KmtaaL66yH4= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=pks.im; spf=pass smtp.mailfrom=pks.im; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b=GdV8ZvKa; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b=kQYKejub; arc=none smtp.client-ip=103.168.172.146 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=pks.im Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=pks.im Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b="GdV8ZvKa"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="kQYKejub" Received: from phl-compute-04.internal (phl-compute-04.phl.internal [10.202.2.44]) by mailfout.phl.internal (Postfix) with ESMTP id 85875138017C; Wed, 16 Apr 2025 06:02:28 -0400 (EDT) Received: from phl-mailfrontend-02 ([10.202.2.163]) by phl-compute-04.internal (MEProxy); Wed, 16 Apr 2025 06:02:28 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pks.im; h=cc:cc :content-transfer-encoding:content-type:content-type:date:date :from:from:in-reply-to:in-reply-to:message-id:mime-version :references:reply-to:subject:subject:to:to; s=fm2; t=1744797748; x=1744884148; bh=44jiKLSD83Xu+/YPtcE1QjeP/cSIoSODcbtVDMG9VCk=; b= GdV8ZvKacyK7FD4jByi7vkSVkfl6sQaAx8Jpe8NcNYXfrV9XVHhS34kXKu6e2xb3 m6EcvjJeOrhbMD1xireFtuFDoyd2nunoLOLx9YBwBhUwNlCxsbwbZUCzbbankH4f UXrWfEHeJg8mmvb4sLyKvuvrfiAIFWJkl5id1i8jfMHMskVmQqJEA5m5dZuGLfKM uFlBJAsfGpFg4FFh673p2GbNMtnkSB3fdq/CF5FB7899LIXUP8mBe95RiHZmxAWU oYRl0VjxfKIhdNre676PQZJvUPDbXBIOVZcHqxRSdOuExfZFZWsMHGM33swATXJ2 wixY1TMzXfZ/0eJp+srAug== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-transfer-encoding :content-type:content-type:date:date:feedback-id:feedback-id :from:from:in-reply-to:in-reply-to:message-id:mime-version :references:reply-to:subject:subject:to:to:x-me-proxy :x-me-sender:x-me-sender:x-sasl-enc; s=fm2; t=1744797748; x= 1744884148; bh=44jiKLSD83Xu+/YPtcE1QjeP/cSIoSODcbtVDMG9VCk=; b=k QYKejubMZEyGVrbeHJHXTHcJAV7gMVKGefYvrqnmpfw5IWnFIRL/nr8XMWCHkJBq FoiYaf+z+5Ydwnhf0fqfpeTslqpj4enkOlixDNRKGFI20MYInaZQvM6Ile4c05w4 uSqhNu37GlKn08fqqERk1ReH9dYZuUny+DLbSAEv8WFCJtYR9gOG0zXnyMfKqylJ 4hCYO8KZUaycpImPNriCR1CvoDrcs9/0JaK60FKj95H4d9B59yo52iTsyL7fUumR T256mRiwTlmI4HLoljm4dq1hg9Bm4YXrYv2HflG0fCNr9MA7oyGmhx6uWDP9dDhG DAtqIq6NkawTG+v1yLLzQ== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeefvddrtddtgddvvdeitdelucetufdoteggodetrf dotffvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdggtfgfnhhsuhgsshgtrhhisggv pdfurfetoffkrfgpnffqhgenuceurghilhhouhhtmecufedttdenucesvcftvggtihhpih gvnhhtshculddquddttddmnecujfgurhephfffufggtgfgkfhfjgfvvefosehtjeertder tdejnecuhfhrohhmpefrrghtrhhitghkucfuthgvihhnhhgrrhguthcuoehpshesphhksh drihhmqeenucggtffrrghtthgvrhhnpefgvdefjeeugfegieevveffhfekueehfeeliefh uddtteekheffhffhkefggeekieenucffohhmrghinhepfhhlrghgshdrhhgvlhhpnecuve hluhhsthgvrhfuihiivgeptdenucfrrghrrghmpehmrghilhhfrhhomhepphhssehpkhhs rdhimhdpnhgspghrtghpthhtohepkedpmhhouggvpehsmhhtphhouhhtpdhrtghpthhtoh epshiivgguvghrrdguvghvsehgmhgrihhlrdgtohhmpdhrtghpthhtoheplhdrshdrrhes figvsgdruggvpdhrtghpthhtohepphgvfhhfsehpvghffhdrnhgvthdprhgtphhtthhope hphhhilhhlihhprdifohhougduvdefsehgmhgrihhlrdgtohhmpdhrtghpthhtohepshht ohhlvggvsehgmhgrihhlrdgtohhmpdhrtghpthhtohepghhithesvhhgvghrrdhkvghrnh gvlhdrohhrghdprhgtphhtthhopehtmhiisehpohgsohigrdgtohhmpdhrtghpthhtohep ghhlrghusghithiisehphhihshhikhdrfhhuqdgsvghrlhhinhdruggv X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Wed, 16 Apr 2025 06:02:26 -0400 (EDT) Received: by vm-mail (OpenSMTPD) with ESMTPSA id 9d1fa254 (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Wed, 16 Apr 2025 10:02:23 +0000 (UTC) From: Patrick Steinhardt Date: Wed, 16 Apr 2025 12:02:10 +0200 Subject: [PATCH v3 1/7] global: use designated initializers for options Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Message-Id: <20250416-b4-pks-parse-options-integers-v3-1-d390746bea79@pks.im> References: <20250416-b4-pks-parse-options-integers-v3-0-d390746bea79@pks.im> In-Reply-To: <20250416-b4-pks-parse-options-integers-v3-0-d390746bea79@pks.im> To: git@vger.kernel.org Cc: John Paul Adrian Glaubitz , Todd Zullinger , =?utf-8?q?Ren=C3=A9_Scharfe?= , =?utf-8?q?SZEDER_G=C3=A1bor?= , Derrick Stolee , Jeff King , Phillip Wood X-Mailer: b4 0.14.2 While we expose macros for most of our different option types understood by the "parse-options" subsystem, not every combination of fields that has one as that would otherwise quickly lead to an explosion of macros. Instead, we just initialize structures manually for those variants of fields that don't have a macro. Callsites that open-code these structure initialization don't use designated initializers though and instead just provide values for each of the fields that they want to initialize. This has three significant downsides: - Callsites need to specify all values up to the last field that they care about. This often includes fields that should simply be left at their default zero-initialized state, which adds distraction. - Any reader not deeply familiar with the layout of the structure has a hard time figuring out what the respective initializers mean. - Reordering or introducing new fields in the middle of the structure is impossible without adapting all callsites. Convert all sites to instead use designated initializers, which we have started using in our codebase quite a while ago. This allows us to skip any default-initialized fields, gives the reader context by specifying the field names and allows us to reorder or introduce new fields where we want to. Signed-off-by: Patrick Steinhardt --- archive.c | 35 ++++++++--- builtin/am.c | 28 ++++++--- builtin/clone.c | 13 ++++- builtin/commit-tree.c | 12 +++- builtin/commit.c | 62 +++++++++++++++----- builtin/config.c | 13 ++++- builtin/describe.c | 24 ++++++-- builtin/fetch.c | 10 +++- builtin/fmt-merge-msg.c | 25 +++++--- builtin/gc.c | 12 +++- builtin/grep.c | 14 +++-- builtin/init-db.c | 13 +++-- builtin/ls-remote.c | 11 +++- builtin/merge.c | 37 +++++++++--- builtin/read-tree.c | 11 +++- builtin/rebase.c | 25 ++++++-- builtin/revert.c | 12 +++- builtin/show-branch.c | 12 +++- builtin/tag.c | 23 ++++++-- builtin/update-index.c | 131 +++++++++++++++++++++++++++++------------- builtin/write-tree.c | 12 ++-- diff.c | 13 +++-- ref-filter.h | 15 +++-- t/helper/test-parse-options.c | 38 +++++++++--- 24 files changed, 443 insertions(+), 158 deletions(-) diff --git a/archive.c b/archive.c index 8be4e7ac8db..67bba3cd301 100644 --- a/archive.c +++ b/archive.c @@ -650,20 +650,37 @@ static int parse_archive_args(int argc, const char **argv, OPT_STRING(0, "format", &format, N_("fmt"), N_("archive format")), OPT_STRING(0, "prefix", &base, N_("prefix"), N_("prepend prefix to each pathname in the archive")), - { OPTION_CALLBACK, 0, "add-file", args, N_("file"), - N_("add untracked file to archive"), 0, add_file_cb, - (intptr_t)&base }, - { OPTION_CALLBACK, 0, "add-virtual-file", args, - N_("path:content"), N_("add untracked file to archive"), 0, - add_file_cb, (intptr_t)&base }, + { + .type = OPTION_CALLBACK, + .long_name = "add-file", + .value = args, + .argh = N_("file"), + .help = N_("add untracked file to archive"), + .callback = add_file_cb, + .defval = (intptr_t) &base, + }, + { + .type = OPTION_CALLBACK, + .long_name = "add-virtual-file", + .value = args, + .argh = N_("path:content"), + .help = N_("add untracked file to archive"), + .callback = add_file_cb, + .defval = (intptr_t) &base, + }, OPT_STRING('o', "output", &output, N_("file"), N_("write the archive to this file")), OPT_BOOL(0, "worktree-attributes", &worktree_attributes, N_("read .gitattributes in working directory")), OPT__VERBOSE(&verbose, N_("report archived files on stderr")), - { OPTION_STRING, 0, "mtime", &mtime_option, N_("time"), - N_("set modification time of archive entries"), - PARSE_OPT_NONEG }, + { + .type = OPTION_STRING, + .long_name = "mtime", + .value = &mtime_option, + .argh = N_("time"), + .help = N_("set modification time of archive entries"), + .flags = PARSE_OPT_NONEG, + }, OPT_NUMBER_CALLBACK(&compression_level, N_("set compression level"), number_callback), OPT_GROUP(""), diff --git a/builtin/am.c b/builtin/am.c index 3b61bd4c333..4afb519830f 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -2400,11 +2400,16 @@ int cmd_am(int argc, OPT_CMDMODE(0, "quit", &resume_mode, N_("abort the patching operation but keep HEAD where it is"), RESUME_QUIT), - { OPTION_CALLBACK, 0, "show-current-patch", &resume_mode, - "(diff|raw)", - N_("show the patch being applied"), - PARSE_OPT_CMDMODE | PARSE_OPT_OPTARG | PARSE_OPT_NONEG | PARSE_OPT_LITERAL_ARGHELP, - parse_opt_show_current_patch, RESUME_SHOW_PATCH_RAW }, + { + .type = OPTION_CALLBACK, + .long_name = "show-current-patch", + .value = &resume_mode, + .argh = "(diff|raw)", + .help = N_("show the patch being applied"), + .flags = PARSE_OPT_CMDMODE | PARSE_OPT_OPTARG | PARSE_OPT_NONEG | PARSE_OPT_LITERAL_ARGHELP, + .callback = parse_opt_show_current_patch, + .defval = RESUME_SHOW_PATCH_RAW, + }, OPT_CMDMODE(0, "retry", &resume_mode, N_("try to apply current patch again"), RESUME_APPLY), @@ -2417,9 +2422,16 @@ int cmd_am(int argc, OPT_BOOL(0, "ignore-date", &state.ignore_date, N_("use current timestamp for author date")), OPT_RERERE_AUTOUPDATE(&state.allow_rerere_autoupdate), - { OPTION_STRING, 'S', "gpg-sign", &state.sign_commit, N_("key-id"), - N_("GPG-sign commits"), - PARSE_OPT_OPTARG, NULL, (intptr_t) "" }, + { + .type = OPTION_STRING, + .short_name = 'S', + .long_name = "gpg-sign", + .value = &state.sign_commit, + .argh = N_("key-id"), + .help = N_("GPG-sign commits"), + .flags = PARSE_OPT_OPTARG, + .defval = (intptr_t) "", + }, OPT_CALLBACK_F(0, "empty", &state.empty_type, "(stop|drop|keep)", N_("how to handle empty patches"), PARSE_OPT_NONEG, am_option_parse_empty), diff --git a/builtin/clone.c b/builtin/clone.c index 88276e5b7ab..9c3547f41e3 100644 --- a/builtin/clone.c +++ b/builtin/clone.c @@ -930,9 +930,16 @@ int cmd_clone(int argc, N_("don't use local hardlinks, always copy")), OPT_BOOL('s', "shared", &option_shared, N_("setup as shared repository")), - { OPTION_CALLBACK, 0, "recurse-submodules", &option_recurse_submodules, - N_("pathspec"), N_("initialize submodules in the clone"), - PARSE_OPT_OPTARG, recurse_submodules_cb, (intptr_t)"." }, + { + .type = OPTION_CALLBACK, + .long_name = "recurse-submodules", + .value = &option_recurse_submodules, + .argh = N_("pathspec"), + .help = N_("initialize submodules in the clone"), + .flags = PARSE_OPT_OPTARG, + .callback = recurse_submodules_cb, + .defval = (intptr_t)".", + }, OPT_ALIAS(0, "recursive", "recurse-submodules"), OPT_INTEGER('j', "jobs", &max_jobs, N_("number of submodules cloned in parallel")), diff --git a/builtin/commit-tree.c b/builtin/commit-tree.c index 38457600a4e..c787133d004 100644 --- a/builtin/commit-tree.c +++ b/builtin/commit-tree.c @@ -111,8 +111,16 @@ int cmd_commit_tree(int argc, OPT_CALLBACK_F('F', NULL, &buffer, N_("file"), N_("read commit log message from file"), PARSE_OPT_NONEG, parse_file_arg_callback), - { OPTION_STRING, 'S', "gpg-sign", &sign_commit, N_("key-id"), - N_("GPG sign commit"), PARSE_OPT_OPTARG, NULL, (intptr_t) "" }, + { + .type = OPTION_STRING, + .short_name = 'S', + .long_name = "gpg-sign", + .value = &sign_commit, + .argh = N_("key-id"), + .help = N_("GPG sign commit"), + .flags = PARSE_OPT_OPTARG, + .defval = (intptr_t) "", + }, OPT_END() }; int ret; diff --git a/builtin/commit.c b/builtin/commit.c index 2f459682221..66bd91fd523 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -1542,17 +1542,34 @@ struct repository *repo UNUSED) STATUS_FORMAT_LONG), OPT_BOOL('z', "null", &s.null_termination, N_("terminate entries with NUL")), - { OPTION_STRING, 'u', "untracked-files", &untracked_files_arg, - N_("mode"), - N_("show untracked files, optional modes: all, normal, no. (Default: all)"), - PARSE_OPT_OPTARG, NULL, (intptr_t)"all" }, - { OPTION_STRING, 0, "ignored", &ignored_arg, - N_("mode"), - N_("show ignored files, optional modes: traditional, matching, no. (Default: traditional)"), - PARSE_OPT_OPTARG, NULL, (intptr_t)"traditional" }, - { OPTION_STRING, 0, "ignore-submodules", &ignore_submodule_arg, N_("when"), - N_("ignore changes to submodules, optional when: all, dirty, untracked. (Default: all)"), - PARSE_OPT_OPTARG, NULL, (intptr_t)"all" }, + { + .type = OPTION_STRING, + .short_name = 'u', + .long_name = "untracked-files", + .value = &untracked_files_arg, + .argh = N_("mode"), + .help = N_("show untracked files, optional modes: all, normal, no. (Default: all)"), + .flags = PARSE_OPT_OPTARG, + .defval = (intptr_t)"all", + }, + { + .type = OPTION_STRING, + .long_name = "ignored", + .value = &ignored_arg, + .argh = N_("mode"), + .help = N_("show ignored files, optional modes: traditional, matching, no. (Default: traditional)"), + .flags = PARSE_OPT_OPTARG, + .defval = (intptr_t)"traditional", + }, + { + .type = OPTION_STRING, + .long_name = "ignore-submodules", + .value = &ignore_submodule_arg, + .argh = N_("when"), + .help = N_("ignore changes to submodules, optional when: all, dirty, untracked. (Default: all)"), + .flags = PARSE_OPT_OPTARG, + .defval = (intptr_t)"all", + }, OPT_COLUMN(0, "column", &s.colopts, N_("list untracked files in columns")), OPT_BOOL(0, "no-renames", &no_renames, N_("do not detect renames")), OPT_CALLBACK_F('M', "find-renames", &rename_score_arg, @@ -1688,8 +1705,16 @@ int cmd_commit(int argc, OPT_BOOL('e', "edit", &edit_flag, N_("force edit of commit")), OPT_CLEANUP(&cleanup_arg), OPT_BOOL(0, "status", &include_status, N_("include status in commit message template")), - { OPTION_STRING, 'S', "gpg-sign", &sign_commit, N_("key-id"), - N_("GPG sign commit"), PARSE_OPT_OPTARG, NULL, (intptr_t) "" }, + { + .type = OPTION_STRING, + .short_name = 'S', + .long_name = "gpg-sign", + .value = &sign_commit, + .argh = N_("key-id"), + .help = N_("GPG sign commit"), + .flags = PARSE_OPT_OPTARG, + .defval = (intptr_t) "", + }, /* end commit message options */ OPT_GROUP(N_("Commit contents options")), @@ -1714,7 +1739,16 @@ int cmd_commit(int argc, N_("terminate entries with NUL")), OPT_BOOL(0, "amend", &amend, N_("amend previous commit")), OPT_BOOL(0, "no-post-rewrite", &no_post_rewrite, N_("bypass post-rewrite hook")), - { OPTION_STRING, 'u', "untracked-files", &untracked_files_arg, N_("mode"), N_("show untracked files, optional modes: all, normal, no. (Default: all)"), PARSE_OPT_OPTARG, NULL, (intptr_t)"all" }, + { + .type = OPTION_STRING, + .short_name = 'u', + .long_name = "untracked-files", + .value = &untracked_files_arg, + .argh = N_("mode"), + .help = N_("show untracked files, optional modes: all, normal, no. (Default: all)"), + .flags = PARSE_OPT_OPTARG, + .defval = (intptr_t)"all", + }, OPT_PATHSPEC_FROM_FILE(&pathspec_from_file), OPT_PATHSPEC_FILE_NUL(&pathspec_file_nul), /* end commit contents options */ diff --git a/builtin/config.c b/builtin/config.c index 53a90094e31..f70d6354772 100644 --- a/builtin/config.c +++ b/builtin/config.c @@ -131,9 +131,16 @@ struct config_display_options { #define TYPE_COLOR 6 #define TYPE_BOOL_OR_STR 7 -#define OPT_CALLBACK_VALUE(s, l, v, h, i) \ - { OPTION_CALLBACK, (s), (l), (v), NULL, (h), PARSE_OPT_NOARG | \ - PARSE_OPT_NONEG, option_parse_type, (i) } +#define OPT_CALLBACK_VALUE(s, l, v, h, i) { \ + .type = OPTION_CALLBACK, \ + .short_name = (s), \ + .long_name = (l), \ + .value = (v), \ + .help = (h), \ + .flags = PARSE_OPT_NOARG | PARSE_OPT_NONEG, \ + .callback = option_parse_type, \ + .defval = (i), \ +} static int option_parse_type(const struct option *opt, const char *arg, int unset) diff --git a/builtin/describe.c b/builtin/describe.c index e2e73f3d757..2da9f4fed01 100644 --- a/builtin/describe.c +++ b/builtin/describe.c @@ -601,12 +601,24 @@ int cmd_describe(int argc, N_("do not consider tags matching ")), OPT_BOOL(0, "always", &always, N_("show abbreviated commit object as fallback")), - {OPTION_STRING, 0, "dirty", &dirty, N_("mark"), - N_("append on dirty working tree (default: \"-dirty\")"), - PARSE_OPT_OPTARG, NULL, (intptr_t) "-dirty"}, - {OPTION_STRING, 0, "broken", &broken, N_("mark"), - N_("append on broken working tree (default: \"-broken\")"), - PARSE_OPT_OPTARG, NULL, (intptr_t) "-broken"}, + { + .type = OPTION_STRING, + .long_name = "dirty", + .value = &dirty, + .argh = N_("mark"), + .help = N_("append on dirty working tree (default: \"-dirty\")"), + .flags = PARSE_OPT_OPTARG, + .defval = (intptr_t) "-dirty", + }, + { + .type = OPTION_STRING, + .long_name = "broken", + .value = &broken, + .argh = N_("mark"), + .help = N_("append on broken working tree (default: \"-broken\")"), + .flags = PARSE_OPT_OPTARG, + .defval = (intptr_t) "-broken", + }, OPT_END(), }; diff --git a/builtin/fetch.c b/builtin/fetch.c index 02af5054690..3a5159d9e69 100644 --- a/builtin/fetch.c +++ b/builtin/fetch.c @@ -2359,8 +2359,14 @@ int cmd_fetch(int argc, OPT_SET_INT_F(0, "refetch", &refetch, N_("re-fetch without negotiating common commits"), 1, PARSE_OPT_NONEG), - { OPTION_STRING, 0, "submodule-prefix", &submodule_prefix, N_("dir"), - N_("prepend this to submodule path output"), PARSE_OPT_HIDDEN }, + { + .type = OPTION_STRING, + .long_name = "submodule-prefix", + .value = &submodule_prefix, + .argh = N_("dir"), + .help = N_("prepend this to submodule path output"), + .flags = PARSE_OPT_HIDDEN, + }, OPT_CALLBACK_F(0, "recurse-submodules-default", &recurse_submodules_default, N_("on-demand"), N_("default for recursive fetching of submodules " diff --git a/builtin/fmt-merge-msg.c b/builtin/fmt-merge-msg.c index 189cd1096a0..240cdb474bc 100644 --- a/builtin/fmt-merge-msg.c +++ b/builtin/fmt-merge-msg.c @@ -20,13 +20,24 @@ int cmd_fmt_merge_msg(int argc, char *into_name = NULL; int shortlog_len = -1; struct option options[] = { - { OPTION_INTEGER, 0, "log", &shortlog_len, N_("n"), - N_("populate log with at most entries from shortlog"), - PARSE_OPT_OPTARG, NULL, DEFAULT_MERGE_LOG_LEN }, - { OPTION_INTEGER, 0, "summary", &shortlog_len, N_("n"), - N_("alias for --log (deprecated)"), - PARSE_OPT_OPTARG | PARSE_OPT_HIDDEN, NULL, - DEFAULT_MERGE_LOG_LEN }, + { + .type = OPTION_INTEGER, + .long_name = "log", + .value = &shortlog_len, + .argh = N_("n"), + .help = N_("populate log with at most entries from shortlog"), + .flags = PARSE_OPT_OPTARG, + .defval = DEFAULT_MERGE_LOG_LEN, + }, + { + .type = OPTION_INTEGER, + .long_name = "summary", + .value = &shortlog_len, + .argh = N_("n"), + .help = N_("alias for --log (deprecated)"), + .flags = PARSE_OPT_OPTARG | PARSE_OPT_HIDDEN, + .defval = DEFAULT_MERGE_LOG_LEN, + }, OPT_STRING('m', "message", &message, N_("text"), N_("use as start of message")), OPT_STRING(0, "into-name", &into_name, N_("name"), diff --git a/builtin/gc.c b/builtin/gc.c index 99431fd4674..6707a26bc6e 100644 --- a/builtin/gc.c +++ b/builtin/gc.c @@ -699,9 +699,15 @@ struct repository *repo UNUSED) int ret; struct option builtin_gc_options[] = { OPT__QUIET(&quiet, N_("suppress progress reporting")), - { OPTION_STRING, 0, "prune", &prune_expire_arg, N_("date"), - N_("prune unreferenced objects"), - PARSE_OPT_OPTARG, NULL, (intptr_t)prune_expire_arg }, + { + .type = OPTION_STRING, + .long_name = "prune", + .value = &prune_expire_arg, + .argh = N_("date"), + .help = N_("prune unreferenced objects"), + .flags = PARSE_OPT_OPTARG, + .defval = (intptr_t)prune_expire_arg, + }, OPT_BOOL(0, "cruft", &cfg.cruft_packs, N_("pack unreferenced objects separately")), OPT_MAGNITUDE(0, "max-cruft-size", &cfg.max_cruft_size, N_("with --cruft, limit the size of new cruft packs")), diff --git a/builtin/grep.c b/builtin/grep.c index d1427290f77..c4869733e1b 100644 --- a/builtin/grep.c +++ b/builtin/grep.c @@ -1017,10 +1017,16 @@ int cmd_grep(int argc, OPT_BOOL(0, "all-match", &opt.all_match, N_("show only matches from files that match all patterns")), OPT_GROUP(""), - { OPTION_STRING, 'O', "open-files-in-pager", &show_in_pager, - N_("pager"), N_("show matching files in the pager"), - PARSE_OPT_OPTARG | PARSE_OPT_NOCOMPLETE, - NULL, (intptr_t)default_pager }, + { + .type = OPTION_STRING, + .short_name = 'O', + .long_name = "open-files-in-pager", + .value = &show_in_pager, + .argh = N_("pager"), + .help = N_("show matching files in the pager"), + .flags = PARSE_OPT_OPTARG | PARSE_OPT_NOCOMPLETE, + .defval = (intptr_t)default_pager, + }, OPT_BOOL_F(0, "ext-grep", &external_grep_allowed__ignored, N_("allow calling of grep(1) (ignored by this build)"), PARSE_OPT_NOCOMPLETE), diff --git a/builtin/init-db.c b/builtin/init-db.c index 196dccdd77a..4a950e44d8d 100644 --- a/builtin/init-db.c +++ b/builtin/init-db.c @@ -93,10 +93,15 @@ int cmd_init_db(int argc, N_("directory from which templates will be used")), OPT_SET_INT(0, "bare", &is_bare_repository_cfg, N_("create a bare repository"), 1), - { OPTION_CALLBACK, 0, "shared", &init_shared_repository, - N_("permissions"), - N_("specify that the git repository is to be shared amongst several users"), - PARSE_OPT_OPTARG | PARSE_OPT_NONEG, shared_callback, 0}, + { + .type = OPTION_CALLBACK, + .long_name = "shared", + .value = &init_shared_repository, + .argh = N_("permissions"), + .help = N_("specify that the git repository is to be shared amongst several users"), + .flags = PARSE_OPT_OPTARG | PARSE_OPT_NONEG, + .callback = shared_callback + }, OPT_BIT('q', "quiet", &flags, N_("be quiet"), INIT_DB_QUIET), OPT_STRING(0, "separate-git-dir", &real_git_dir, N_("gitdir"), N_("separate git dir from working tree")), diff --git a/builtin/ls-remote.c b/builtin/ls-remote.c index 42f34e12361..01a4d4daa1f 100644 --- a/builtin/ls-remote.c +++ b/builtin/ls-remote.c @@ -67,9 +67,14 @@ int cmd_ls_remote(int argc, OPT__QUIET(&quiet, N_("do not print remote URL")), OPT_STRING(0, "upload-pack", &uploadpack, N_("exec"), N_("path of git-upload-pack on the remote host")), - { OPTION_STRING, 0, "exec", &uploadpack, N_("exec"), - N_("path of git-upload-pack on the remote host"), - PARSE_OPT_HIDDEN }, + { + .type = OPTION_STRING, + .long_name = "exec", + .value = &uploadpack, + .argh = N_("exec"), + .help = N_("path of git-upload-pack on the remote host"), + .flags = PARSE_OPT_HIDDEN, + }, OPT_BIT('t', "tags", &flags, N_("limit to tags"), REF_TAGS), OPT_BIT('b', "branches", &flags, N_("limit to branches"), REF_BRANCHES), OPT_BIT_F('h', "heads", &flags, diff --git a/builtin/merge.c b/builtin/merge.c index ba9faf126aa..21787d45165 100644 --- a/builtin/merge.c +++ b/builtin/merge.c @@ -250,9 +250,15 @@ static struct option builtin_merge_options[] = { OPT_BOOL(0, "stat", &show_diffstat, N_("show a diffstat at the end of the merge")), OPT_BOOL(0, "summary", &show_diffstat, N_("(synonym to --stat)")), - { OPTION_INTEGER, 0, "log", &shortlog_len, N_("n"), - N_("add (at most ) entries from shortlog to merge commit message"), - PARSE_OPT_OPTARG, NULL, DEFAULT_MERGE_LOG_LEN }, + { + .type = OPTION_INTEGER, + .long_name = "log", + .value = &shortlog_len, + .argh = N_("n"), + .help = N_("add (at most ) entries from shortlog to merge commit message"), + .flags = PARSE_OPT_OPTARG, + .defval = DEFAULT_MERGE_LOG_LEN, + }, OPT_BOOL(0, "squash", &squash, N_("create a single commit instead of doing a merge")), OPT_BOOL(0, "commit", &option_commit, @@ -274,9 +280,16 @@ static struct option builtin_merge_options[] = { OPT_CALLBACK('m', "message", &merge_msg, N_("message"), N_("merge commit message (for a non-fast-forward merge)"), option_parse_message), - { OPTION_LOWLEVEL_CALLBACK, 'F', "file", &merge_msg, N_("path"), - N_("read message from file"), PARSE_OPT_NONEG, - NULL, 0, option_read_message }, + { + .type = OPTION_LOWLEVEL_CALLBACK, + .short_name = 'F', + .long_name = "file", + .value = &merge_msg, + .argh = N_("path"), + .help = N_("read message from file"), + .flags = PARSE_OPT_NONEG, + .ll_callback = option_read_message, + }, OPT_STRING(0, "into-name", &into_name, N_("name"), N_("use instead of the real target")), OPT__VERBOSITY(&verbosity), @@ -289,8 +302,16 @@ static struct option builtin_merge_options[] = { OPT_BOOL(0, "allow-unrelated-histories", &allow_unrelated_histories, N_("allow merging unrelated histories")), OPT_SET_INT(0, "progress", &show_progress, N_("force progress reporting"), 1), - { OPTION_STRING, 'S', "gpg-sign", &sign_commit, N_("key-id"), - N_("GPG sign commit"), PARSE_OPT_OPTARG, NULL, (intptr_t) "" }, + { + .type = OPTION_STRING, + .short_name = 'S', + .long_name = "gpg-sign", + .value = &sign_commit, + .argh = N_("key-id"), + .help = N_("GPG sign commit"), + .flags = PARSE_OPT_OPTARG, + .defval = (intptr_t) "", + }, OPT_AUTOSTASH(&autostash), OPT_BOOL(0, "overwrite-ignore", &overwrite_ignore, N_("update ignored files (default)")), OPT_BOOL(0, "signoff", &signoff, N_("add a Signed-off-by trailer")), diff --git a/builtin/read-tree.c b/builtin/read-tree.c index d2a807a828b..a8f352f7cd9 100644 --- a/builtin/read-tree.c +++ b/builtin/read-tree.c @@ -135,9 +135,14 @@ int cmd_read_tree(int argc, N_("3-way merge in presence of adds and removes")), OPT_BOOL(0, "reset", &opts.reset, N_("same as -m, but discard unmerged entries")), - { OPTION_STRING, 0, "prefix", &opts.prefix, N_("/"), - N_("read the tree into the index under /"), - PARSE_OPT_NONEG }, + { + .type = OPTION_STRING, + .long_name = "prefix", + .value = &opts.prefix, + .argh = N_("/"), + .help = N_("read the tree into the index under /"), + .flags = PARSE_OPT_NONEG, + }, OPT_BOOL('u', NULL, &opts.update, N_("update working tree with merge result")), OPT_CALLBACK_F(0, "exclude-per-directory", &opts, diff --git a/builtin/rebase.c b/builtin/rebase.c index d4715ed35d7..d4083350090 100644 --- a/builtin/rebase.c +++ b/builtin/rebase.c @@ -1122,9 +1122,15 @@ int cmd_rebase(int argc, OPT_BIT('v', "verbose", &options.flags, N_("display a diffstat of what changed upstream"), REBASE_NO_QUIET | REBASE_VERBOSE | REBASE_DIFFSTAT), - {OPTION_NEGBIT, 'n', "no-stat", &options.flags, NULL, - N_("do not show diffstat of what changed upstream"), - PARSE_OPT_NOARG, NULL, REBASE_DIFFSTAT }, + { + .type = OPTION_NEGBIT, + .short_name = 'n', + .long_name = "no-stat", + .value = &options.flags, + .help = N_("do not show diffstat of what changed upstream"), + .flags = PARSE_OPT_NOARG, + .defval = REBASE_DIFFSTAT, + }, OPT_BOOL(0, "signoff", &options.signoff, N_("add a Signed-off-by trailer to each commit")), OPT_BOOL(0, "committer-date-is-author-date", @@ -1190,9 +1196,16 @@ int cmd_rebase(int argc, OPT_BOOL(0, "update-refs", &options.update_refs, N_("update branches that point to commits " "that are being rebased")), - { OPTION_STRING, 'S', "gpg-sign", &gpg_sign, N_("key-id"), - N_("GPG-sign commits"), - PARSE_OPT_OPTARG, NULL, (intptr_t) "" }, + { + .type = OPTION_STRING, + .short_name = 'S', + .long_name = "gpg-sign", + .value = &gpg_sign, + .argh = N_("key-id"), + .help = N_("GPG-sign commits"), + .flags = PARSE_OPT_OPTARG, + .defval = (intptr_t) "", + }, OPT_AUTOSTASH(&options.autostash), OPT_STRING_LIST('x', "exec", &options.exec, N_("exec"), N_("add exec lines after each commit of the " diff --git a/builtin/revert.c b/builtin/revert.c index aca6c293cdf..4f5ef975494 100644 --- a/builtin/revert.c +++ b/builtin/revert.c @@ -132,8 +132,16 @@ static int run_sequencer(int argc, const char **argv, const char *prefix, OPT_STRING(0, "strategy", &strategy, N_("strategy"), N_("merge strategy")), OPT_STRVEC('X', "strategy-option", &opts->xopts, N_("option"), N_("option for merge strategy")), - { OPTION_STRING, 'S', "gpg-sign", &gpg_sign, N_("key-id"), - N_("GPG sign commit"), PARSE_OPT_OPTARG, NULL, (intptr_t) "" }, + { + .type = OPTION_STRING, + .short_name = 'S', + .long_name = "gpg-sign", + .value = &gpg_sign, + .argh = N_("key-id"), + .help = N_("GPG sign commit"), + .flags = PARSE_OPT_OPTARG, + .defval = (intptr_t) "", + }, OPT_END() }; struct option *options = base_options; diff --git a/builtin/show-branch.c b/builtin/show-branch.c index fce6b404e92..dab37019d29 100644 --- a/builtin/show-branch.c +++ b/builtin/show-branch.c @@ -667,9 +667,15 @@ int cmd_show_branch(int ac, N_("show remote-tracking branches")), OPT__COLOR(&showbranch_use_color, N_("color '*!+-' corresponding to the branch")), - { OPTION_INTEGER, 0, "more", &extra, N_("n"), - N_("show more commits after the common ancestor"), - PARSE_OPT_OPTARG, NULL, (intptr_t)1 }, + { + .type = OPTION_INTEGER, + .long_name = "more", + .value = &extra, + .argh = N_("n"), + .help = N_("show more commits after the common ancestor"), + .flags = PARSE_OPT_OPTARG, + .defval = 1, + }, OPT_SET_INT(0, "list", &extra, N_("synonym to more=-1"), -1), OPT_BOOL(0, "no-name", &no_name, N_("suppress naming strings")), OPT_BOOL(0, "current", &with_current_branch, diff --git a/builtin/tag.c b/builtin/tag.c index d3e0943b734..b266f12bb48 100644 --- a/builtin/tag.c +++ b/builtin/tag.c @@ -479,9 +479,15 @@ int cmd_tag(int argc, int edit_flag = 0; struct option options[] = { OPT_CMDMODE('l', "list", &cmdmode, N_("list tag names"), 'l'), - { OPTION_INTEGER, 'n', NULL, &filter.lines, N_("n"), - N_("print lines of each tag message"), - PARSE_OPT_OPTARG, NULL, 1 }, + { + .type = OPTION_INTEGER, + .short_name = 'n', + .value = &filter.lines, + .argh = N_("n"), + .help = N_("print lines of each tag message"), + .flags = PARSE_OPT_OPTARG, + .defval = 1, + }, OPT_CMDMODE('d', "delete", &cmdmode, N_("delete tags"), 'd'), OPT_CMDMODE('v', "verify", &cmdmode, N_("verify tags"), 'v'), @@ -513,9 +519,14 @@ int cmd_tag(int argc, N_("do not output a newline after empty formatted refs")), OPT_REF_SORT(&sorting_options), { - OPTION_CALLBACK, 0, "points-at", &filter.points_at, N_("object"), - N_("print only tags of the object"), PARSE_OPT_LASTARG_DEFAULT, - parse_opt_object_name, (intptr_t) "HEAD" + .type = OPTION_CALLBACK, + .long_name = "points-at", + .value = &filter.points_at, + .argh = N_("object"), + .help = N_("print only tags of the object"), + .flags = PARSE_OPT_LASTARG_DEFAULT, + .callback = parse_opt_object_name, + .defval = (intptr_t) "HEAD", }, OPT_STRING( 0 , "format", &format.format, N_("format"), N_("format to use for the output")), diff --git a/builtin/update-index.c b/builtin/update-index.c index b2f6b1a3fbb..ee64b022679 100644 --- a/builtin/update-index.c +++ b/builtin/update-index.c @@ -964,29 +964,51 @@ int cmd_update_index(int argc, N_("like --refresh, but ignore assume-unchanged setting"), PARSE_OPT_NOARG | PARSE_OPT_NONEG, really_refresh_callback), - {OPTION_LOWLEVEL_CALLBACK, 0, "cacheinfo", NULL, - N_(",,"), - N_("add the specified entry to the index"), - PARSE_OPT_NOARG | /* disallow --cacheinfo= form */ - PARSE_OPT_NONEG | PARSE_OPT_LITERAL_ARGHELP, - NULL, 0, - cacheinfo_callback}, + { + .type = OPTION_LOWLEVEL_CALLBACK, + .long_name = "cacheinfo", + .argh = N_(",,"), + .help = N_("add the specified entry to the index"), + .flags = PARSE_OPT_NOARG | /* disallow --cacheinfo= form */ + PARSE_OPT_NONEG | PARSE_OPT_LITERAL_ARGHELP, + .ll_callback = cacheinfo_callback, + }, OPT_CALLBACK_F(0, "chmod", &set_executable_bit, "(+|-)x", N_("override the executable bit of the listed files"), PARSE_OPT_NONEG, chmod_callback), - {OPTION_SET_INT, 0, "assume-unchanged", &mark_valid_only, NULL, - N_("mark files as \"not changing\""), - PARSE_OPT_NOARG | PARSE_OPT_NONEG, NULL, MARK_FLAG}, - {OPTION_SET_INT, 0, "no-assume-unchanged", &mark_valid_only, NULL, - N_("clear assumed-unchanged bit"), - PARSE_OPT_NOARG | PARSE_OPT_NONEG, NULL, UNMARK_FLAG}, - {OPTION_SET_INT, 0, "skip-worktree", &mark_skip_worktree_only, NULL, - N_("mark files as \"index-only\""), - PARSE_OPT_NOARG | PARSE_OPT_NONEG, NULL, MARK_FLAG}, - {OPTION_SET_INT, 0, "no-skip-worktree", &mark_skip_worktree_only, NULL, - N_("clear skip-worktree bit"), - PARSE_OPT_NOARG | PARSE_OPT_NONEG, NULL, UNMARK_FLAG}, + { + .type = OPTION_SET_INT, + .long_name = "assume-unchanged", + .value = &mark_valid_only, + .help = N_("mark files as \"not changing\""), + .flags = PARSE_OPT_NOARG | PARSE_OPT_NONEG, + .defval = MARK_FLAG, + }, + { + .type = OPTION_SET_INT, + .long_name = "no-assume-unchanged", + .value = &mark_valid_only, + .help = N_("clear assumed-unchanged bit"), + .flags = PARSE_OPT_NOARG | PARSE_OPT_NONEG, + .defval = UNMARK_FLAG, + }, + { + .type = OPTION_SET_INT, + .long_name = "skip-worktree", + .value = &mark_skip_worktree_only, + .help = N_("mark files as \"index-only\""), + .flags = PARSE_OPT_NOARG | PARSE_OPT_NONEG, + .defval = MARK_FLAG, + }, + { + .type = OPTION_SET_INT, + .long_name = "no-skip-worktree", + .value = &mark_skip_worktree_only, + .help = N_("clear skip-worktree bit"), + .flags = PARSE_OPT_NOARG | PARSE_OPT_NONEG, + .defval = UNMARK_FLAG, + }, OPT_BOOL(0, "ignore-skip-worktree-entries", &ignore_skip_worktree_entries, N_("do not touch index-only entries")), OPT_SET_INT(0, "info-only", &info_only, @@ -995,22 +1017,39 @@ int cmd_update_index(int argc, N_("remove named paths even if present in worktree"), 1), OPT_BOOL('z', NULL, &nul_term_line, N_("with --stdin: input lines are terminated by null bytes")), - {OPTION_LOWLEVEL_CALLBACK, 0, "stdin", &read_from_stdin, NULL, - N_("read list of paths to be updated from standard input"), - PARSE_OPT_NONEG | PARSE_OPT_NOARG, - NULL, 0, stdin_callback}, - {OPTION_LOWLEVEL_CALLBACK, 0, "index-info", &nul_term_line, NULL, - N_("add entries from standard input to the index"), - PARSE_OPT_NONEG | PARSE_OPT_NOARG, - NULL, 0, stdin_cacheinfo_callback}, - {OPTION_LOWLEVEL_CALLBACK, 0, "unresolve", &has_errors, NULL, - N_("repopulate stages #2 and #3 for the listed paths"), - PARSE_OPT_NONEG | PARSE_OPT_NOARG, - NULL, 0, unresolve_callback}, - {OPTION_LOWLEVEL_CALLBACK, 'g', "again", &has_errors, NULL, - N_("only update entries that differ from HEAD"), - PARSE_OPT_NONEG | PARSE_OPT_NOARG, - NULL, 0, reupdate_callback}, + { + .type = OPTION_LOWLEVEL_CALLBACK, + .long_name = "stdin", + .value = &read_from_stdin, + .help = N_("read list of paths to be updated from standard input"), + .flags = PARSE_OPT_NONEG | PARSE_OPT_NOARG, + .ll_callback = stdin_callback, + }, + { + .type = OPTION_LOWLEVEL_CALLBACK, + .long_name = "index-info", + .value = &nul_term_line, + .help = N_("add entries from standard input to the index"), + .flags = PARSE_OPT_NONEG | PARSE_OPT_NOARG, + .ll_callback = stdin_cacheinfo_callback, + }, + { + .type = OPTION_LOWLEVEL_CALLBACK, + .long_name = "unresolve", + .value = &has_errors, + .help = N_("repopulate stages #2 and #3 for the listed paths"), + .flags = PARSE_OPT_NONEG | PARSE_OPT_NOARG, + .ll_callback = unresolve_callback, + }, + { + .type = OPTION_LOWLEVEL_CALLBACK, + .short_name = 'g', + .long_name = "again", + .value = &has_errors, + .help = N_("only update entries that differ from HEAD"), + .flags = PARSE_OPT_NONEG | PARSE_OPT_NOARG, + .ll_callback = reupdate_callback, + }, OPT_BIT(0, "ignore-missing", &refresh_args.flags, N_("ignore files missing from worktree"), REFRESH_IGNORE_MISSING), @@ -1036,12 +1075,22 @@ int cmd_update_index(int argc, N_("write out the index even if is not flagged as changed"), 1), OPT_BOOL(0, "fsmonitor", &fsmonitor, N_("enable or disable file system monitor")), - {OPTION_SET_INT, 0, "fsmonitor-valid", &mark_fsmonitor_only, NULL, - N_("mark files as fsmonitor valid"), - PARSE_OPT_NOARG | PARSE_OPT_NONEG, NULL, MARK_FLAG}, - {OPTION_SET_INT, 0, "no-fsmonitor-valid", &mark_fsmonitor_only, NULL, - N_("clear fsmonitor valid bit"), - PARSE_OPT_NOARG | PARSE_OPT_NONEG, NULL, UNMARK_FLAG}, + { + .type = OPTION_SET_INT, + .long_name = "fsmonitor-valid", + .value = &mark_fsmonitor_only, + .help = N_("mark files as fsmonitor valid"), + .flags = PARSE_OPT_NOARG | PARSE_OPT_NONEG, + .defval = MARK_FLAG, + }, + { + .type = OPTION_SET_INT, + .long_name = "no-fsmonitor-valid", + .value = &mark_fsmonitor_only, + .help = N_("clear fsmonitor valid bit"), + .flags = PARSE_OPT_NOARG | PARSE_OPT_NONEG, + .defval = UNMARK_FLAG, + }, OPT_END() }; diff --git a/builtin/write-tree.c b/builtin/write-tree.c index 43f233e69b0..5a8dc377ec0 100644 --- a/builtin/write-tree.c +++ b/builtin/write-tree.c @@ -31,10 +31,14 @@ int cmd_write_tree(int argc, WRITE_TREE_MISSING_OK), OPT_STRING(0, "prefix", &tree_prefix, N_("/"), N_("write tree object for a subdirectory ")), - { OPTION_BIT, 0, "ignore-cache-tree", &flags, NULL, - N_("only useful for debugging"), - PARSE_OPT_HIDDEN | PARSE_OPT_NOARG, NULL, - WRITE_TREE_IGNORE_CACHE_TREE }, + { + .type = OPTION_BIT, + .long_name = "ignore-cache-tree", + .value = &flags, + .help = N_("only useful for debugging"), + .flags = PARSE_OPT_HIDDEN | PARSE_OPT_NOARG, + .defval = WRITE_TREE_IGNORE_CACHE_TREE, + }, OPT_END() }; diff --git a/diff.c b/diff.c index 08f5e00a2cc..f2fcc7f3c22 100644 --- a/diff.c +++ b/diff.c @@ -5892,10 +5892,15 @@ struct option *add_diff_options(const struct option *opts, OPT_CALLBACK_F(0, "diff-filter", options, N_("[(A|C|D|M|R|T|U|X|B)...[*]]"), N_("select files by diff type"), PARSE_OPT_NONEG, diff_opt_diff_filter), - { OPTION_CALLBACK, 0, "output", options, N_(""), - N_("output to a specific file"), - PARSE_OPT_NONEG, NULL, 0, diff_opt_output }, - + { + .type = OPTION_CALLBACK, + .long_name = "output", + .value = options, + .argh = N_(""), + .help = N_("output to a specific file"), + .flags = PARSE_OPT_NONEG, + .ll_callback = diff_opt_output, + }, OPT_END() }; diff --git a/ref-filter.h b/ref-filter.h index 013d4cfa64b..c98c4fbd4c1 100644 --- a/ref-filter.h +++ b/ref-filter.h @@ -114,11 +114,16 @@ struct ref_format { } /* Macros for checking --merged and --no-merged options */ -#define _OPT_MERGED_NO_MERGED(option, filter, h) \ - { OPTION_CALLBACK, 0, option, (filter), N_("commit"), (h), \ - PARSE_OPT_LASTARG_DEFAULT | PARSE_OPT_NONEG, \ - parse_opt_merge_filter, (intptr_t) "HEAD" \ - } +#define _OPT_MERGED_NO_MERGED(option, filter, h) { \ + .type = OPTION_CALLBACK, \ + .long_name = option, \ + .value = (filter), \ + .argh = N_("commit"), \ + .help = (h), \ + .flags = PARSE_OPT_LASTARG_DEFAULT | PARSE_OPT_NONEG, \ + .callback = parse_opt_merge_filter, \ + .defval = (intptr_t) "HEAD", \ +} #define OPT_MERGED(f, h) _OPT_MERGED_NO_MERGED("merged", f, h) #define OPT_NO_MERGED(f, h) _OPT_MERGED_NO_MERGED("no-merged", f, h) diff --git a/t/helper/test-parse-options.c b/t/helper/test-parse-options.c index bfe45ec68b0..997f55fd45b 100644 --- a/t/helper/test-parse-options.c +++ b/t/helper/test-parse-options.c @@ -124,8 +124,15 @@ int cmd__parse_options(int argc, const char **argv) struct option options[] = { OPT_BOOL(0, "yes", &boolean, "get a boolean"), OPT_BOOL('D', "no-doubt", &boolean, "begins with 'no-'"), - { OPTION_SET_INT, 'B', "no-fear", &boolean, NULL, - "be brave", PARSE_OPT_NOARG | PARSE_OPT_NONEG, NULL, 1 }, + { + .type = OPTION_SET_INT, + .short_name = 'B', + .long_name = "no-fear", + .value = &boolean, + .help = "be brave", + .flags = PARSE_OPT_NOARG | PARSE_OPT_NONEG, + .defval = 1, + }, OPT_COUNTUP('b', "boolean", &boolean, "increment by one"), OPT_BIT('4', "or4", &boolean, "bitwise-or boolean with ...0100", 4), @@ -155,12 +162,27 @@ int cmd__parse_options(int argc, const char **argv) OPT_GROUP("Magic arguments"), OPT_NUMBER_CALLBACK(&integer, "set integer to NUM", number_callback), - { OPTION_COUNTUP, '+', NULL, &boolean, NULL, "same as -b", - PARSE_OPT_NOARG | PARSE_OPT_NONEG | PARSE_OPT_NODASH }, - { OPTION_COUNTUP, 0, "ambiguous", &ambiguous, NULL, - "positive ambiguity", PARSE_OPT_NOARG | PARSE_OPT_NONEG }, - { OPTION_COUNTUP, 0, "no-ambiguous", &ambiguous, NULL, - "negative ambiguity", PARSE_OPT_NOARG | PARSE_OPT_NONEG }, + { + .type = OPTION_COUNTUP, + .short_name = '+', + .value = &boolean, + .help = "same as -b", + .flags = PARSE_OPT_NOARG | PARSE_OPT_NONEG | PARSE_OPT_NODASH, + }, + { + .type = OPTION_COUNTUP, + .long_name = "ambiguous", + .value = &ambiguous, + .help = "positive ambiguity", + .flags = PARSE_OPT_NOARG | PARSE_OPT_NONEG, + }, + { + .type = OPTION_COUNTUP, + .long_name = "no-ambiguous", + .value = &ambiguous, + .help = "negative ambiguity", + .flags = PARSE_OPT_NOARG | PARSE_OPT_NONEG, + }, OPT_GROUP("Standard options"), OPT__ABBREV(&abbrev), OPT__VERBOSE(&verbose, "be verbose"), From patchwork Wed Apr 16 10:02:11 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 14053611 Received: from fhigh-a6-smtp.messagingengine.com (fhigh-a6-smtp.messagingengine.com [103.168.172.157]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id D8C2923315F for ; Wed, 16 Apr 2025 10:02:29 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=103.168.172.157 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1744797751; cv=none; b=uIzQH+B0g0pxbFF+BNnWi7AvZcR47DwHi7ejKl0/b+i9m6I+2ou+Hsc5ViZG/H1vxyKRCZUIHlwmkAxgJXMADqm+/GBS8Mg7rMaZMudP/6J52K+QvD3S6V7lT1qnNO3yXSgV/P2cOEDRr4mIlegaTfEwJp+qEXWck8mAoTsGzUk= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1744797751; c=relaxed/simple; bh=X52QNf6dbTU5OV8bZx0IJ3q9QIyGl81wL5E3yumdarE=; h=From:Date:Subject:MIME-Version:Content-Type:Message-Id:References: In-Reply-To:To:Cc; b=Gs/swYFC5OxjNfo4QfNp1qSUps9o610ySL/Gp3bs4lBtfo4FO3nzFvVC/8vnGvyYgD67amZjGFNQQfBnRU5Oi5Nb0SBTGr0LyYhitXvbaI7mJdHERTeU6gyYGyStBP8U6Ilq0io3DZtwFvWyTfDBbQ5wu6YYNNwYGyZxIyZsGP4= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=pks.im; spf=pass smtp.mailfrom=pks.im; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b=p74Oc40q; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b=hpgdpixP; arc=none smtp.client-ip=103.168.172.157 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=pks.im Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=pks.im Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b="p74Oc40q"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="hpgdpixP" Received: from phl-compute-01.internal (phl-compute-01.phl.internal [10.202.2.41]) by mailfhigh.phl.internal (Postfix) with ESMTP id BAE1211400FE; Wed, 16 Apr 2025 06:02:28 -0400 (EDT) Received: from phl-mailfrontend-01 ([10.202.2.162]) by phl-compute-01.internal (MEProxy); Wed, 16 Apr 2025 06:02:28 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pks.im; h=cc:cc :content-transfer-encoding:content-type:content-type:date:date :from:from:in-reply-to:in-reply-to:message-id:mime-version :references:reply-to:subject:subject:to:to; s=fm2; t=1744797748; x=1744884148; bh=hR3ek4FdtgdKdX+D35Uqr/VegY1XzVUfhO/Caooe/Kw=; b= p74Oc40qHVTb8pst37lq5cGyqQESC89qVbWXo2NUSFYgfug5SWWIGh+75ss78eNf I9HZSiHAMkiSU424yJeLVPFV245rWXMcV2tw8tlfnBprCpRY/4dQVtefmWgNmSr3 ILxMJgAP4zBj0TV7tlHuTwvzFYYwTBGYaFijw3PeFYVuduBedzvg67Wh1KHDvfwr N7XDlyY0JP/WNy3EBt21HqVni30zN6O29EEK8yoLIzKfR7sG7JZunLzMG8UaKzTE arefvJydHL4020zM8KHRyUmRTHhDKLO3d0RGizNeNyEedkkojMuGmf7iBMV6qgHF +anzZ3B85ToVLL4vmMwAYg== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-transfer-encoding :content-type:content-type:date:date:feedback-id:feedback-id :from:from:in-reply-to:in-reply-to:message-id:mime-version :references:reply-to:subject:subject:to:to:x-me-proxy :x-me-sender:x-me-sender:x-sasl-enc; s=fm2; t=1744797748; x= 1744884148; bh=hR3ek4FdtgdKdX+D35Uqr/VegY1XzVUfhO/Caooe/Kw=; b=h pgdpixPQ4ru/ZTv0VmE18yBf+dwUSmE2a0C8YpeFAugbMlmggbi7hgyqrlhpUUHm dxeU1lfj2BfyxonSqG8i+7thxF5LDoRnm12Omi4CL4/r92EBmVX+VKZpkBLv+ITB uyntxepiCDP7lCEDc4h2f9vGpx5p3xIVTljNMqvscs56C6+oiWwpp7L6aibFvzqD 0dCRJqEBt8cOZOCp9DeBP53PCzia3FF/pp3bQjbnV84U6ALQ2OPhWwk6rPjv9m5L etYlFFlZZgVtSwbQTQFIE36XWM6j7nWgHDrhzgmqWy2Hxko6XuJEGGf7642jVUi4 iXEQEMC27qau8AlOtgeFQ== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeefvddrtddtgddvvdeitdekucetufdoteggodetrf dotffvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdggtfgfnhhsuhgsshgtrhhisggv pdfurfetoffkrfgpnffqhgenuceurghilhhouhhtmecufedttdenucesvcftvggtihhpih gvnhhtshculddquddttddmnecujfgurhephfffufggtgfgkfhfjgfvvefosehtjeertder tdejnecuhfhrohhmpefrrghtrhhitghkucfuthgvihhnhhgrrhguthcuoehpshesphhksh drihhmqeenucggtffrrghtthgvrhhnpeffueeiudejvdekheeuvdekfeffiedvueelteek udehjeetkeegvddugfdtgfeileenucevlhhushhtvghrufhiiigvpedtnecurfgrrhgrmh epmhgrihhlfhhrohhmpehpshesphhkshdrihhmpdhnsggprhgtphhtthhopeekpdhmohgu vgepshhmthhpohhuthdprhgtphhtthhopehtmhiisehpohgsohigrdgtohhmpdhrtghpth htohepphgvfhhfsehpvghffhdrnhgvthdprhgtphhtthhopehsiigvuggvrhdruggvvhes ghhmrghilhdrtghomhdprhgtphhtthhopehsthholhgvvgesghhmrghilhdrtghomhdprh gtphhtthhopehgihhtsehvghgvrhdrkhgvrhhnvghlrdhorhhgpdhrtghpthhtohepghhl rghusghithiisehphhihshhikhdrfhhuqdgsvghrlhhinhdruggvpdhrtghpthhtohepph hhihhllhhiphdrfihoohguuddvfeesghhmrghilhdrtghomhdprhgtphhtthhopehlrdhs rdhrseifvggsrdguvg X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Wed, 16 Apr 2025 06:02:27 -0400 (EDT) Received: by vm-mail (OpenSMTPD) with ESMTPSA id 111a3863 (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Wed, 16 Apr 2025 10:02:24 +0000 (UTC) From: Patrick Steinhardt Date: Wed, 16 Apr 2025 12:02:11 +0200 Subject: [PATCH v3 2/7] parse-options: check for overflow when parsing integers Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Message-Id: <20250416-b4-pks-parse-options-integers-v3-2-d390746bea79@pks.im> References: <20250416-b4-pks-parse-options-integers-v3-0-d390746bea79@pks.im> In-Reply-To: <20250416-b4-pks-parse-options-integers-v3-0-d390746bea79@pks.im> To: git@vger.kernel.org Cc: John Paul Adrian Glaubitz , Todd Zullinger , =?utf-8?q?Ren=C3=A9_Scharfe?= , =?utf-8?q?SZEDER_G=C3=A1bor?= , Derrick Stolee , Jeff King , Phillip Wood X-Mailer: b4 0.14.2 We use `strtol()` to parse the argument of `OPTION_INTEGER` options. And while we do check that the argument was fully parsed, we don't check `errno` at all and thus may not notice cases where `strtol()` fails. Most importantly, this includes the case where the parsed integer does not fit into a `long` at all. The consequence is that we'll happily continue with an invalid value. Fix the bug by checking `errno`. Note that this change alone is not sufficient to detect all possible overflows: `strtol()` returns a `long`, but we end up assigning the value to an `int` and will thus truncate the value. This will be fixed in subsequent patches. Signed-off-by: Patrick Steinhardt --- parse-options.c | 10 +++++++++- t/t0040-parse-options.sh | 6 ++++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/parse-options.c b/parse-options.c index 35fbb3b0d63..e8c08e55e02 100644 --- a/parse-options.c +++ b/parse-options.c @@ -185,12 +185,20 @@ static enum parse_opt_result do_get_value(struct parse_opt_ctx_t *p, if (!*arg) return error(_("%s expects a numerical value"), optname(opt, flags)); + + errno = 0; *(int *)opt->value = strtol(arg, (char **)&s, 10); if (*s) return error(_("%s expects a numerical value"), optname(opt, flags)); - return 0; + if (errno == ERANGE) + return error(_("value %s for %s not in range [%"PRIdMAX",%"PRIdMAX"]"), + arg, optname(opt, flags), (intmax_t)LONG_MIN, (intmax_t)LONG_MAX); + if (errno) + return error_errno(_("value %s for %s cannot be parsed"), + arg, optname(opt, flags)); + return 0; case OPTION_MAGNITUDE: if (unset) { *(unsigned long *)opt->value = 0; diff --git a/t/t0040-parse-options.sh b/t/t0040-parse-options.sh index 2fe3522305f..5eb1feb61b4 100755 --- a/t/t0040-parse-options.sh +++ b/t/t0040-parse-options.sh @@ -783,4 +783,10 @@ test_expect_success 'magnitude with units but no numbers' ' test_must_be_empty out ' +test_expect_success 'overflowing integer' ' + test_must_fail test-tool parse-options --integer 9223372036854775808 >out 2>err && + test_grep "value .* for option .* not in range" err && + test_must_be_empty out +' + test_done From patchwork Wed Apr 16 10:02:12 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 14053614 Received: from fout-a3-smtp.messagingengine.com (fout-a3-smtp.messagingengine.com [103.168.172.146]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 2ECFB238144 for ; Wed, 16 Apr 2025 10:02:30 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=103.168.172.146 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1744797753; cv=none; b=nyKR552jq55MTzFiQd8bREw741CRPJC9JVGhnvpgvBFj0f8v6hHrZXArgXN8wNr34wg8GCRB7vilQd63yQ7DA4VvBdHSnwEChVc7zDRNeCa2ml/MXvgn0kP6VHtSufVEAn4auUwotqFbNee7qZi6s1RCrQoOCi5KHPTr4Isy9WE= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1744797753; c=relaxed/simple; bh=LJss9KCOiT6NaDFpz73huf9SdSDIPLaMSPhNx6awWqo=; h=From:Date:Subject:MIME-Version:Content-Type:Message-Id:References: In-Reply-To:To:Cc; b=mJTNFM+3YbmPCq1/wIl7jcduRairJ0IA+KDzweuKB34+9pd6luHnGylC5VgoTmtzg/JSu2sXTy/86urPGlOFnJSRSd6L8D+aQ16vi8zQTUVO6YOGFo4avhg4EQMbkrUAE89W0J+RU2zlEZnyrQufMmbvOZnSt1yJqeSgLdx20zE= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=pks.im; spf=pass smtp.mailfrom=pks.im; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b=J2IDhagi; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b=NcpzGSjB; arc=none smtp.client-ip=103.168.172.146 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=pks.im Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=pks.im Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b="J2IDhagi"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="NcpzGSjB" Received: from phl-compute-05.internal (phl-compute-05.phl.internal [10.202.2.45]) by mailfout.phl.internal (Postfix) with ESMTP id 2893C1380196; Wed, 16 Apr 2025 06:02:30 -0400 (EDT) Received: from phl-mailfrontend-02 ([10.202.2.163]) by phl-compute-05.internal (MEProxy); Wed, 16 Apr 2025 06:02:30 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pks.im; h=cc:cc :content-transfer-encoding:content-type:content-type:date:date :from:from:in-reply-to:in-reply-to:message-id:mime-version :references:reply-to:subject:subject:to:to; s=fm2; t=1744797750; x=1744884150; bh=6RLnQdulVmAYPteG/ekQ6rKbilmmu2qgG6rOpYkN9OE=; b= J2IDhagiETN5mrvN1nfYBrKo+5DCR1gkOiVBaRFkYqivbnA2JCc6rOx0PN8l8Xq6 Jqh/J9voOl72/v2zDjs6lallPFDyMqYqT+pjmOUVjJWb7gHrUgghQbKZ8FBGLuWC 9wnl+0/98SlG6LKRjM0Qg4wxnk/1A/dbPp8QJwBFS1vzMSvD6RPAb1a6V1w1Zl71 OlxUwXx2T7tKHeY0J4tMbUc0sQlkn3r/Hf3bvWOnTPeiOdBdePQ95Kn+/vzegcsv wVTAZfs93QYgTl+4Yo+pbUjhJElpCZb0qTORa5iwzAHuAmn0jl420bDgdM/rztqR 2x+IxNQAg9XXMUVQLfy0OA== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-transfer-encoding :content-type:content-type:date:date:feedback-id:feedback-id :from:from:in-reply-to:in-reply-to:message-id:mime-version :references:reply-to:subject:subject:to:to:x-me-proxy :x-me-sender:x-me-sender:x-sasl-enc; s=fm2; t=1744797750; x= 1744884150; bh=6RLnQdulVmAYPteG/ekQ6rKbilmmu2qgG6rOpYkN9OE=; b=N cpzGSjBMuTufq4SBUvNFN1zT79pwt7hg9K0GEWI1KbX8F3iEL6Fp926b0LTD6d/Z M67NAArhp2/67k7M1YEK2GHKdFLYN+lSOXwVPSDmnGmbj57TvtsweRFGD+YU5rLF NweMN5e21zIwVGPnnPUR7nznP299OqxJYeTWqynrPJWqIAixRaEX1NluEm/VO2nD 6hg1qZGTfh2852jb4+PTlMQsk76HWiSTFXvEH+YRmuyiZxjRktbVeB4Dpfgpwg9O /tFhk+x6umiE0reviuNzAMzmNk9JTVSmIJdZCyyIz5S/2LB35T7Cp2vPv/TpRP2Z jx8wVTJfpK+bM1bAuLk0A== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeefvddrtddtgddvvdeitdekucetufdoteggodetrf dotffvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdggtfgfnhhsuhgsshgtrhhisggv pdfurfetoffkrfgpnffqhgenuceurghilhhouhhtmecufedttdenucesvcftvggtihhpih gvnhhtshculddquddttddmnecujfgurhephfffufggtgfgkfhfjgfvvefosehtjeertder tdejnecuhfhrohhmpefrrghtrhhitghkucfuthgvihhnhhgrrhguthcuoehpshesphhksh drihhmqeenucggtffrrghtthgvrhhnpeffueeiudejvdekheeuvdekfeffiedvueelteek udehjeetkeegvddugfdtgfeileenucevlhhushhtvghrufhiiigvpedtnecurfgrrhgrmh epmhgrihhlfhhrohhmpehpshesphhkshdrihhmpdhnsggprhgtphhtthhopeekpdhmohgu vgepshhmthhpohhuthdprhgtphhtthhopehsthholhgvvgesghhmrghilhdrtghomhdprh gtphhtthhopehgihhtsehvghgvrhdrkhgvrhhnvghlrdhorhhgpdhrtghpthhtoheplhdr shdrrhesfigvsgdruggvpdhrtghpthhtohepghhlrghusghithiisehphhihshhikhdrfh huqdgsvghrlhhinhdruggvpdhrtghpthhtohepphhhihhllhhiphdrfihoohguuddvfees ghhmrghilhdrtghomhdprhgtphhtthhopehtmhiisehpohgsohigrdgtohhmpdhrtghpth htohepshiivgguvghrrdguvghvsehgmhgrihhlrdgtohhmpdhrtghpthhtohepphgvfhhf sehpvghffhdrnhgvth X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Wed, 16 Apr 2025 06:02:28 -0400 (EDT) Received: by vm-mail (OpenSMTPD) with ESMTPSA id ff6c225b (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Wed, 16 Apr 2025 10:02:25 +0000 (UTC) From: Patrick Steinhardt Date: Wed, 16 Apr 2025 12:02:12 +0200 Subject: [PATCH v3 3/7] parse-options: introduce precision handling for `OPTION_INTEGER` Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Message-Id: <20250416-b4-pks-parse-options-integers-v3-3-d390746bea79@pks.im> References: <20250416-b4-pks-parse-options-integers-v3-0-d390746bea79@pks.im> In-Reply-To: <20250416-b4-pks-parse-options-integers-v3-0-d390746bea79@pks.im> To: git@vger.kernel.org Cc: John Paul Adrian Glaubitz , Todd Zullinger , =?utf-8?q?Ren=C3=A9_Scharfe?= , =?utf-8?q?SZEDER_G=C3=A1bor?= , Derrick Stolee , Jeff King , Phillip Wood X-Mailer: b4 0.14.2 The `OPTION_INTEGER` option type accepts a signed integer. The type of the underlying integer is a simple `int`, which restricts the range of values accepted by such options. But there is a catch: because the caller provides a pointer to the value via the `.value` field, which is a simple void pointer. This has two consequences: - There is no check whether the passed value is sufficiently long to store the entire range of `int`. This can lead to integer wraparound in the best case and out-of-bounds writes in the worst case. - Even when a caller knows that they want to store a value larger than `INT_MAX` they don't have a way to do so. Funny enough, even if the caller gets everything correct the parsing logic is still insufficient because we use `strtol()` to parse the argument, which returns a `long`. But as that value is implicitly cast when assigning it to the `int` field we may still get invalid results. In practice this doesn't tend to be a huge issue because users typically don't end up passing huge values to most commands. But the parsing logic is demonstrably broken, and it is too easy to get the calling convention wrong. Improve the situation by introducing a new `precision` field into the structure. This field gets assigned automatically by `OPT_INTEGER_F()` and tracks the size of the passed value. Like this it becomes possible for the caller to pass arbitrarily-sized integers and the underlying logic knows to handle it correctly by doing range checks. Furthermore, convert the code to use `strtoimax()` intstead of `strtol()` so that we can also parse values larger than `LONG_MAX`. Note that we do not yet assert signedness of the passed variable, which is another source of bugs. This will be handled in a subsequent commit. Signed-off-by: Patrick Steinhardt --- builtin/fmt-merge-msg.c | 2 ++ builtin/merge.c | 1 + builtin/show-branch.c | 1 + builtin/tag.c | 1 + parse-options.c | 63 +++++++++++++++++++++++++++++-------------- parse-options.h | 6 +++++ t/helper/test-parse-options.c | 3 +++ t/t0040-parse-options.sh | 23 +++++++++++++++- 8 files changed, 79 insertions(+), 21 deletions(-) diff --git a/builtin/fmt-merge-msg.c b/builtin/fmt-merge-msg.c index 240cdb474bc..3b6aac2cf7f 100644 --- a/builtin/fmt-merge-msg.c +++ b/builtin/fmt-merge-msg.c @@ -24,6 +24,7 @@ int cmd_fmt_merge_msg(int argc, .type = OPTION_INTEGER, .long_name = "log", .value = &shortlog_len, + .precision = sizeof(shortlog_len), .argh = N_("n"), .help = N_("populate log with at most entries from shortlog"), .flags = PARSE_OPT_OPTARG, @@ -33,6 +34,7 @@ int cmd_fmt_merge_msg(int argc, .type = OPTION_INTEGER, .long_name = "summary", .value = &shortlog_len, + .precision = sizeof(shortlog_len), .argh = N_("n"), .help = N_("alias for --log (deprecated)"), .flags = PARSE_OPT_OPTARG | PARSE_OPT_HIDDEN, diff --git a/builtin/merge.c b/builtin/merge.c index 21787d45165..9ab10c7db0a 100644 --- a/builtin/merge.c +++ b/builtin/merge.c @@ -254,6 +254,7 @@ static struct option builtin_merge_options[] = { .type = OPTION_INTEGER, .long_name = "log", .value = &shortlog_len, + .precision = sizeof(shortlog_len), .argh = N_("n"), .help = N_("add (at most ) entries from shortlog to merge commit message"), .flags = PARSE_OPT_OPTARG, diff --git a/builtin/show-branch.c b/builtin/show-branch.c index dab37019d29..b549d8c3f5b 100644 --- a/builtin/show-branch.c +++ b/builtin/show-branch.c @@ -671,6 +671,7 @@ int cmd_show_branch(int ac, .type = OPTION_INTEGER, .long_name = "more", .value = &extra, + .precision = sizeof(extra), .argh = N_("n"), .help = N_("show more commits after the common ancestor"), .flags = PARSE_OPT_OPTARG, diff --git a/builtin/tag.c b/builtin/tag.c index b266f12bb48..7597d93c71b 100644 --- a/builtin/tag.c +++ b/builtin/tag.c @@ -483,6 +483,7 @@ int cmd_tag(int argc, .type = OPTION_INTEGER, .short_name = 'n', .value = &filter.lines, + .precision = sizeof(filter.lines), .argh = N_("n"), .help = N_("print lines of each tag message"), .flags = PARSE_OPT_OPTARG, diff --git a/parse-options.c b/parse-options.c index e8c08e55e02..2cb9bd3b5b9 100644 --- a/parse-options.c +++ b/parse-options.c @@ -172,33 +172,56 @@ static enum parse_opt_result do_get_value(struct parse_opt_ctx_t *p, return (*opt->ll_callback)(p, opt, p_arg, p_unset); } case OPTION_INTEGER: + { + intmax_t upper_bound = INTMAX_MAX >> (bitsizeof(intmax_t) - CHAR_BIT * opt->precision); + intmax_t lower_bound = -upper_bound - 1; + intmax_t value; + if (unset) { - *(int *)opt->value = 0; - return 0; - } - if (opt->flags & PARSE_OPT_OPTARG && !p->opt) { - *(int *)opt->value = opt->defval; - return 0; - } - if (get_arg(p, opt, flags, &arg)) + value = 0; + } else if (opt->flags & PARSE_OPT_OPTARG && !p->opt) { + value = opt->defval; + } else if (get_arg(p, opt, flags, &arg)) { return -1; - if (!*arg) + } else if (!*arg) { return error(_("%s expects a numerical value"), optname(opt, flags)); + } else { + errno = 0; + value = strtoimax(arg, (char **)&s, 10); + if (*s) + return error(_("%s expects a numerical value"), + optname(opt, flags)); + if (errno == ERANGE) + return error(_("value %s for %s not in range [%"PRIdMAX",%"PRIdMAX"]"), + arg, optname(opt, flags), lower_bound, upper_bound); + if (errno) + return error_errno(_("value %s for %s cannot be parsed"), + arg, optname(opt, flags)); + } - errno = 0; - *(int *)opt->value = strtol(arg, (char **)&s, 10); - if (*s) - return error(_("%s expects a numerical value"), - optname(opt, flags)); - if (errno == ERANGE) + if (value < lower_bound || value > upper_bound) return error(_("value %s for %s not in range [%"PRIdMAX",%"PRIdMAX"]"), - arg, optname(opt, flags), (intmax_t)LONG_MIN, (intmax_t)LONG_MAX); - if (errno) - return error_errno(_("value %s for %s cannot be parsed"), - arg, optname(opt, flags)); + arg, optname(opt, flags), lower_bound, upper_bound); - return 0; + switch (opt->precision) { + case 1: + *(int8_t *)opt->value = value; + return 0; + case 2: + *(int16_t *)opt->value = value; + return 0; + case 4: + *(int32_t *)opt->value = value; + return 0; + case 8: + *(int64_t *)opt->value = value; + return 0; + default: + BUG("invalid precision for option %s", + optname(opt, flags)); + } + } case OPTION_MAGNITUDE: if (unset) { *(unsigned long *)opt->value = 0; diff --git a/parse-options.h b/parse-options.h index 997ffbee805..8db96402c4d 100644 --- a/parse-options.h +++ b/parse-options.h @@ -92,6 +92,10 @@ typedef int parse_opt_subcommand_fn(int argc, const char **argv, * `value`:: * stores pointers to the values to be filled. * + * `precision`:: + * precision of the integer pointed to by `value` in number of bytes. Should + * typically be its `sizeof()`. + * * `argh`:: * token to explain the kind of argument this option wants. Does not * begin in capital letter, and does not end with a full stop. @@ -151,6 +155,7 @@ struct option { int short_name; const char *long_name; void *value; + size_t precision; const char *argh; const char *help; @@ -214,6 +219,7 @@ struct option { .short_name = (s), \ .long_name = (l), \ .value = (v), \ + .precision = sizeof(*v), \ .argh = N_("n"), \ .help = (h), \ .flags = (f), \ diff --git a/t/helper/test-parse-options.c b/t/helper/test-parse-options.c index 997f55fd45b..b1275dfade4 100644 --- a/t/helper/test-parse-options.c +++ b/t/helper/test-parse-options.c @@ -120,6 +120,7 @@ int cmd__parse_options(int argc, const char **argv) }; struct string_list expect = STRING_LIST_INIT_NODUP; struct string_list list = STRING_LIST_INIT_NODUP; + int16_t i16 = 0; struct option options[] = { OPT_BOOL(0, "yes", &boolean, "get a boolean"), @@ -139,6 +140,7 @@ int cmd__parse_options(int argc, const char **argv) OPT_NEGBIT(0, "neg-or4", &boolean, "same as --no-or4", 4), OPT_GROUP(""), OPT_INTEGER('i', "integer", &integer, "get a integer"), + OPT_INTEGER(0, "i16", &i16, "get a 16 bit integer"), OPT_INTEGER('j', NULL, &integer, "get a integer, too"), OPT_MAGNITUDE('m', "magnitude", &magnitude, "get a magnitude"), OPT_SET_INT(0, "set23", &integer, "set integer to 23", 23), @@ -210,6 +212,7 @@ int cmd__parse_options(int argc, const char **argv) } show(&expect, &ret, "boolean: %d", boolean); show(&expect, &ret, "integer: %d", integer); + show(&expect, &ret, "i16: %"PRIdMAX, (intmax_t) i16); show(&expect, &ret, "magnitude: %lu", magnitude); show(&expect, &ret, "timestamp: %"PRItime, timestamp); show(&expect, &ret, "string: %s", string ? string : "(not set)"); diff --git a/t/t0040-parse-options.sh b/t/t0040-parse-options.sh index 5eb1feb61b4..95951436cda 100755 --- a/t/t0040-parse-options.sh +++ b/t/t0040-parse-options.sh @@ -22,6 +22,7 @@ usage: test-tool parse-options -i, --[no-]integer get a integer + --[no-]i16 get a 16 bit integer -j get a integer, too -m, --magnitude get a magnitude --[no-]set23 set integer to 23 @@ -136,6 +137,7 @@ test_expect_success 'OPT_MAGNITUDE() 3giga' ' cat >expect <<\EOF boolean: 2 integer: 1729 +i16: 0 magnitude: 16384 timestamp: 0 string: 123 @@ -156,6 +158,7 @@ test_expect_success 'short options' ' cat >expect <<\EOF boolean: 2 integer: 1729 +i16: 9000 magnitude: 16384 timestamp: 0 string: 321 @@ -167,7 +170,7 @@ file: prefix/fi.le EOF test_expect_success 'long options' ' - test-tool parse-options --boolean --integer 1729 --magnitude 16k \ + test-tool parse-options --boolean --integer 1729 --i16 9000 --magnitude 16k \ --boolean --string2=321 --verbose --verbose --no-dry-run \ --abbrev=10 --file fi.le --obsolete \ >output 2>output.err && @@ -179,6 +182,7 @@ test_expect_success 'abbreviate to something longer than SHA1 length' ' cat >expect <<-EOF && boolean: 0 integer: 0 + i16: 0 magnitude: 0 timestamp: 0 string: (not set) @@ -253,6 +257,7 @@ test_expect_success 'superfluous value provided: cmdmode' ' cat >expect <<\EOF boolean: 1 integer: 13 +i16: 0 magnitude: 0 timestamp: 0 string: 123 @@ -276,6 +281,7 @@ test_expect_success 'intermingled arguments' ' cat >expect <<\EOF boolean: 0 integer: 2 +i16: 0 magnitude: 0 timestamp: 0 string: (not set) @@ -343,6 +349,7 @@ cat >expect <<\EOF Callback: "four", 0 boolean: 5 integer: 4 +i16: 0 magnitude: 0 timestamp: 0 string: (not set) @@ -368,6 +375,7 @@ test_expect_success 'OPT_CALLBACK() and callback errors work' ' cat >expect <<\EOF boolean: 1 integer: 23 +i16: 0 magnitude: 0 timestamp: 0 string: (not set) @@ -447,6 +455,7 @@ test_expect_success 'OPT_NUMBER_CALLBACK() works' ' cat >expect <<\EOF boolean: 0 integer: 0 +i16: 0 magnitude: 0 timestamp: 0 string: (not set) @@ -789,4 +798,16 @@ test_expect_success 'overflowing integer' ' test_must_be_empty out ' +test_expect_success 'i16 limits range' ' + test-tool parse-options --i16 32767 >out && + test_grep "i16: 32767" out && + test_must_fail test-tool parse-options --i16 32768 2>err && + test_grep "value 32768 for option .i16. not in range \[-32768,32767\]" err && + + test-tool parse-options --i16 -32768 >out && + test_grep "i16: -32768" out && + test_must_fail test-tool parse-options --i16 -32769 2>err && + test_grep "value -32769 for option .i16. not in range \[-32768,32767\]" err +' + test_done From patchwork Wed Apr 16 10:02:13 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 14053613 Received: from fhigh-a6-smtp.messagingengine.com (fhigh-a6-smtp.messagingengine.com [103.168.172.157]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 3AA8023BCFD for ; Wed, 16 Apr 2025 10:02:30 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=103.168.172.157 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1744797753; cv=none; b=YvrZ62hIkrq0VossyeyOGRlDD8B1H2b9RL4T+yf2l3ML+tg9aHmpKwWnVFhhZbNvuQA9HYeQ6HpKQBmM8o/FRbiorQAJAsV8pit98SCKx6hxuI7+7ZiJJP6qWqhCbip4XApBLk/2BRGT1EzRgC1HXYuzRfc45tR43V/kowmPFHQ= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1744797753; c=relaxed/simple; bh=DWFzKPMh/iozpLmckl5wVvy/NkUHtIsdjVLqJM8l5lo=; h=From:Date:Subject:MIME-Version:Content-Type:Message-Id:References: In-Reply-To:To:Cc; b=rLML4RTmoZSIpc9i3aS1JZcIui4tmrG1qV9Ss0Hl5KsYkbaptHr5+vsiSpM0WBY/rFXtHwOzrmTHLglfy5+QpxkcJh4HVelsesG4o00M9ILVsd/CU8hjVBHPaS4bvR0sVtTeJUIMJDDb16r3mjce8adrDVuXbw/Mgd9lwRLl11Q= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=pks.im; spf=pass smtp.mailfrom=pks.im; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b=H9IfI0Hs; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b=rEAXBqpB; arc=none smtp.client-ip=103.168.172.157 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=pks.im Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=pks.im Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b="H9IfI0Hs"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="rEAXBqpB" Received: from phl-compute-04.internal (phl-compute-04.phl.internal [10.202.2.44]) by mailfhigh.phl.internal (Postfix) with ESMTP id 56B581140102; Wed, 16 Apr 2025 06:02:30 -0400 (EDT) Received: from phl-mailfrontend-01 ([10.202.2.162]) by phl-compute-04.internal (MEProxy); Wed, 16 Apr 2025 06:02:30 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pks.im; h=cc:cc :content-transfer-encoding:content-type:content-type:date:date :from:from:in-reply-to:in-reply-to:message-id:mime-version :references:reply-to:subject:subject:to:to; s=fm2; t=1744797750; x=1744884150; bh=et4S9Kzmlk6L8G9TnDjZZFrJkYovtwHP2u0Lh6hXm0g=; b= H9IfI0HsIom/nyFTTJwRqkpsJ2POfCsslcyGjw7Cyf1/tqWGuJIg3PbkE6yeMzs+ tJ7BX4FRLE6RpMHpTOQShV4fI0cLwJjVBkYgMOT5q7Dqb6rXvYnsYm5ofyDY2uXa Oj2vMLpizc8NCC4y/w1PKeLQnAYGE2wj+bQ7ptRQ/pHxsc0n2BkxTMkyWwjQb857 BKl7LYNbH4tLSY37Ghk/5tuEA4sfqJsQeFnUQ+ddRzxg/CVg0ceGOh9RSAHbPAq8 rJ9mlqDmLI+FJxTWvmYWNmjXBn07sdZbF7Xg8KvusAqELdl+hXpglL8d5r4s+OC1 afbpxy6v/Jhtnb76of6tyA== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-transfer-encoding :content-type:content-type:date:date:feedback-id:feedback-id :from:from:in-reply-to:in-reply-to:message-id:mime-version :references:reply-to:subject:subject:to:to:x-me-proxy :x-me-sender:x-me-sender:x-sasl-enc; s=fm2; t=1744797750; x= 1744884150; bh=et4S9Kzmlk6L8G9TnDjZZFrJkYovtwHP2u0Lh6hXm0g=; b=r EAXBqpBd/Igs6M7UWhXfJnoUc2/CXRjYga+CEU9Ki6xp3lhvCEuMTcqyAnu2GJTT WrNpxMG0odWQjHjobXJu085EkjTqZpgK1D9Im9/ZepcsVI9pPuR9tlcFjKKC3fKA aGBCWhsLugb7avTqjSxkePZi3X8LFkQz8pIunZR8vXN7QW3Hf0hSolWMV51ZfpJm ubgX3SYCji50y1FAgPkoPsPWP3nyBMA5rCtCXNpokDHCBL+vkTsvK6RE17cvIVma +7pM8X8y8aOBqWlJDRj0WMUjxrzXxtZUTr7EL7aw8bvcF3YDeaX3UXX9r76aFHMt 2TVarUsPnjHw9K8orgsAw== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeefvddrtddtgddvvdeitdelucetufdoteggodetrf dotffvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdggtfgfnhhsuhgsshgtrhhisggv pdfurfetoffkrfgpnffqhgenuceurghilhhouhhtmecufedttdenucesvcftvggtihhpih gvnhhtshculddquddttddmnecujfgurhephfffufggtgfgkfhfjgfvvefosehtjeertder tdejnecuhfhrohhmpefrrghtrhhitghkucfuthgvihhnhhgrrhguthcuoehpshesphhksh drihhmqeenucggtffrrghtthgvrhhnpeffueeiudejvdekheeuvdekfeffiedvueelteek udehjeetkeegvddugfdtgfeileenucevlhhushhtvghrufhiiigvpedtnecurfgrrhgrmh epmhgrihhlfhhrohhmpehpshesphhkshdrihhmpdhnsggprhgtphhtthhopeekpdhmohgu vgepshhmthhpohhuthdprhgtphhtthhopehtmhiisehpohgsohigrdgtohhmpdhrtghpth htohepshhtohhlvggvsehgmhgrihhlrdgtohhmpdhrtghpthhtohepghhithesvhhgvghr rdhkvghrnhgvlhdrohhrghdprhgtphhtthhopehlrdhsrdhrseifvggsrdguvgdprhgtph htthhopehglhgruhgsihhtiiesphhhhihsihhkrdhfuhdqsggvrhhlihhnrdguvgdprhgt phhtthhopehphhhilhhlihhprdifohhougduvdefsehgmhgrihhlrdgtohhmpdhrtghpth htohepshiivgguvghrrdguvghvsehgmhgrihhlrdgtohhmpdhrtghpthhtohepphgvfhhf sehpvghffhdrnhgvth X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Wed, 16 Apr 2025 06:02:28 -0400 (EDT) Received: by vm-mail (OpenSMTPD) with ESMTPSA id a62737c9 (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Wed, 16 Apr 2025 10:02:26 +0000 (UTC) From: Patrick Steinhardt Date: Wed, 16 Apr 2025 12:02:13 +0200 Subject: [PATCH v3 4/7] parse-options: introduce precision handling for `OPTION_MAGNITUDE` Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Message-Id: <20250416-b4-pks-parse-options-integers-v3-4-d390746bea79@pks.im> References: <20250416-b4-pks-parse-options-integers-v3-0-d390746bea79@pks.im> In-Reply-To: <20250416-b4-pks-parse-options-integers-v3-0-d390746bea79@pks.im> To: git@vger.kernel.org Cc: John Paul Adrian Glaubitz , Todd Zullinger , =?utf-8?q?Ren=C3=A9_Scharfe?= , =?utf-8?q?SZEDER_G=C3=A1bor?= , Derrick Stolee , Jeff King , Phillip Wood X-Mailer: b4 0.14.2 This commit is the equivalent to the preceding commit, but instead of introducing precision handling for `OPTION_INTEGER` we introduce it for `OPTION_MAGNITUDE`. Signed-off-by: Patrick Steinhardt --- parse-options.c | 45 ++++++++++++++++++++++++++++++++----------- parse-options.h | 1 + t/helper/test-parse-options.c | 3 +++ t/t0040-parse-options.sh | 18 ++++++++++++++++- 4 files changed, 55 insertions(+), 12 deletions(-) diff --git a/parse-options.c b/parse-options.c index 2cb9bd3b5b9..259716efb17 100644 --- a/parse-options.c +++ b/parse-options.c @@ -202,7 +202,7 @@ static enum parse_opt_result do_get_value(struct parse_opt_ctx_t *p, if (value < lower_bound || value > upper_bound) return error(_("value %s for %s not in range [%"PRIdMAX",%"PRIdMAX"]"), - arg, optname(opt, flags), lower_bound, upper_bound); + arg, optname(opt, flags), (intmax_t)lower_bound, (intmax_t)upper_bound); switch (opt->precision) { case 1: @@ -223,21 +223,44 @@ static enum parse_opt_result do_get_value(struct parse_opt_ctx_t *p, } } case OPTION_MAGNITUDE: + { + uintmax_t upper_bound = UINTMAX_MAX >> (bitsizeof(uintmax_t) - CHAR_BIT * opt->precision); + unsigned long value; + if (unset) { - *(unsigned long *)opt->value = 0; - return 0; - } - if (opt->flags & PARSE_OPT_OPTARG && !p->opt) { - *(unsigned long *)opt->value = opt->defval; - return 0; - } - if (get_arg(p, opt, flags, &arg)) + value = 0; + } else if (opt->flags & PARSE_OPT_OPTARG && !p->opt) { + value = opt->defval; + } else if (get_arg(p, opt, flags, &arg)) { return -1; - if (!git_parse_ulong(arg, opt->value)) + } else if (!git_parse_ulong(arg, &value)) { return error(_("%s expects a non-negative integer value" " with an optional k/m/g suffix"), optname(opt, flags)); - return 0; + } + + if (value > upper_bound) + return error(_("value %s for %s not in range [%"PRIuMAX",%"PRIuMAX"]"), + arg, optname(opt, flags), (uintmax_t)0, (uintmax_t)upper_bound); + + switch (opt->precision) { + case 1: + *(uint8_t *)opt->value = value; + return 0; + case 2: + *(uint16_t *)opt->value = value; + return 0; + case 4: + *(uint32_t *)opt->value = value; + return 0; + case 8: + *(uint64_t *)opt->value = value; + return 0; + default: + BUG("invalid precision for option %s", + optname(opt, flags)); + } + } default: BUG("opt->type %d should not happen", opt->type); diff --git a/parse-options.h b/parse-options.h index 8db96402c4d..55c42faa29f 100644 --- a/parse-options.h +++ b/parse-options.h @@ -281,6 +281,7 @@ struct option { .short_name = (s), \ .long_name = (l), \ .value = (v), \ + .precision = sizeof(*v), \ .argh = N_("n"), \ .help = (h), \ .flags = PARSE_OPT_NONEG, \ diff --git a/t/helper/test-parse-options.c b/t/helper/test-parse-options.c index b1275dfade4..46deb4317ef 100644 --- a/t/helper/test-parse-options.c +++ b/t/helper/test-parse-options.c @@ -120,6 +120,7 @@ int cmd__parse_options(int argc, const char **argv) }; struct string_list expect = STRING_LIST_INIT_NODUP; struct string_list list = STRING_LIST_INIT_NODUP; + uint16_t m16 = 0; int16_t i16 = 0; struct option options[] = { @@ -143,6 +144,7 @@ int cmd__parse_options(int argc, const char **argv) OPT_INTEGER(0, "i16", &i16, "get a 16 bit integer"), OPT_INTEGER('j', NULL, &integer, "get a integer, too"), OPT_MAGNITUDE('m', "magnitude", &magnitude, "get a magnitude"), + OPT_MAGNITUDE(0, "m16", &m16, "get a 16 bit magnitude"), OPT_SET_INT(0, "set23", &integer, "set integer to 23", 23), OPT_CMDMODE(0, "mode1", &integer, "set integer to 1 (cmdmode option)", 1), OPT_CMDMODE(0, "mode2", &integer, "set integer to 2 (cmdmode option)", 2), @@ -214,6 +216,7 @@ int cmd__parse_options(int argc, const char **argv) show(&expect, &ret, "integer: %d", integer); show(&expect, &ret, "i16: %"PRIdMAX, (intmax_t) i16); show(&expect, &ret, "magnitude: %lu", magnitude); + show(&expect, &ret, "m16: %"PRIuMAX, (uintmax_t) m16); show(&expect, &ret, "timestamp: %"PRItime, timestamp); show(&expect, &ret, "string: %s", string ? string : "(not set)"); show(&expect, &ret, "abbrev: %d", abbrev); diff --git a/t/t0040-parse-options.sh b/t/t0040-parse-options.sh index 95951436cda..8daaf568485 100755 --- a/t/t0040-parse-options.sh +++ b/t/t0040-parse-options.sh @@ -25,6 +25,7 @@ usage: test-tool parse-options --[no-]i16 get a 16 bit integer -j get a integer, too -m, --magnitude get a magnitude + --m16 get a 16 bit magnitude --[no-]set23 set integer to 23 --mode1 set integer to 1 (cmdmode option) --mode2 set integer to 2 (cmdmode option) @@ -139,6 +140,7 @@ boolean: 2 integer: 1729 i16: 0 magnitude: 16384 +m16: 0 timestamp: 0 string: 123 abbrev: 7 @@ -160,6 +162,7 @@ boolean: 2 integer: 1729 i16: 9000 magnitude: 16384 +m16: 32768 timestamp: 0 string: 321 abbrev: 10 @@ -171,7 +174,7 @@ EOF test_expect_success 'long options' ' test-tool parse-options --boolean --integer 1729 --i16 9000 --magnitude 16k \ - --boolean --string2=321 --verbose --verbose --no-dry-run \ + --m16 32k --boolean --string2=321 --verbose --verbose --no-dry-run \ --abbrev=10 --file fi.le --obsolete \ >output 2>output.err && test_must_be_empty output.err && @@ -184,6 +187,7 @@ test_expect_success 'abbreviate to something longer than SHA1 length' ' integer: 0 i16: 0 magnitude: 0 + m16: 0 timestamp: 0 string: (not set) abbrev: 100 @@ -259,6 +263,7 @@ boolean: 1 integer: 13 i16: 0 magnitude: 0 +m16: 0 timestamp: 0 string: 123 abbrev: 7 @@ -283,6 +288,7 @@ boolean: 0 integer: 2 i16: 0 magnitude: 0 +m16: 0 timestamp: 0 string: (not set) abbrev: 7 @@ -351,6 +357,7 @@ boolean: 5 integer: 4 i16: 0 magnitude: 0 +m16: 0 timestamp: 0 string: (not set) abbrev: 7 @@ -377,6 +384,7 @@ boolean: 1 integer: 23 i16: 0 magnitude: 0 +m16: 0 timestamp: 0 string: (not set) abbrev: 7 @@ -457,6 +465,7 @@ boolean: 0 integer: 0 i16: 0 magnitude: 0 +m16: 0 timestamp: 0 string: (not set) abbrev: 7 @@ -810,4 +819,11 @@ test_expect_success 'i16 limits range' ' test_grep "value -32769 for option .i16. not in range \[-32768,32767\]" err ' +test_expect_success 'm16 limits range' ' + test-tool parse-options --m16 65535 >out && + test_grep "m16: 65535" out && + test_must_fail test-tool parse-options --m16 65536 2>err && + test_grep "value 65536 for option .m16. not in range \[0,65535\]" err +' + test_done From patchwork Wed Apr 16 10:02:14 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 14053615 Received: from fout-a3-smtp.messagingengine.com (fout-a3-smtp.messagingengine.com [103.168.172.146]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 2ED5D238153 for ; Wed, 16 Apr 2025 10:02:31 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=103.168.172.146 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1744797754; cv=none; b=Y5eREM2KBINVmCOrUDncz2EdJ4O+TgNQEaekGzVDcLrNA2HTZEYmHXKAlRQZGnkmbT+63GSH4dIn1hBvHkHNpVL6eoleFSF1IpDB9UPmLRaFC1sZnS8164DPHMQKLUCjpkNuzK4Dp/CXzh/Ymqut+sgdh51SLLrLv6x5ZjlG1to= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1744797754; c=relaxed/simple; bh=NTPOaKtCMGcslMSWT/7obdkeSNMaKkLjSPn2B4UKQ70=; h=From:Date:Subject:MIME-Version:Content-Type:Message-Id:References: In-Reply-To:To:Cc; b=lJjwiavbHGVIPnGQpLUwRFOgVhYPLmYrk/G9IyqM5joIHwHxD4gQaXGpfZspYK3jt9jhdhm+tK9BoIQZHcTqt1I5jNRioRCvZouXO3QU7VO+8rR3qzAmqQnB6Y9SE6HbCAbNJJbV5KD/6npy2BBG90z16kAUkRAcdfNqURXq/XY= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=pks.im; spf=pass smtp.mailfrom=pks.im; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b=UBVH/3MX; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b=SKC183w+; arc=none smtp.client-ip=103.168.172.146 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=pks.im Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=pks.im Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b="UBVH/3MX"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="SKC183w+" Received: from phl-compute-01.internal (phl-compute-01.phl.internal [10.202.2.41]) by mailfout.phl.internal (Postfix) with ESMTP id 5BFDD1380198; Wed, 16 Apr 2025 06:02:30 -0400 (EDT) Received: from phl-mailfrontend-01 ([10.202.2.162]) by phl-compute-01.internal (MEProxy); Wed, 16 Apr 2025 06:02:30 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pks.im; h=cc:cc :content-transfer-encoding:content-type:content-type:date:date :from:from:in-reply-to:in-reply-to:message-id:mime-version :references:reply-to:subject:subject:to:to; s=fm2; t=1744797750; x=1744884150; bh=X0flypOkb5u31lBuemid5c4Nr1z4rinZxjq0JmdZnNI=; b= UBVH/3MXARtma/7aoivwGnneHFBl6PLBbRFaPSUnWvG+7lYeFOGiMlWmMe0GDPWa VWS9IU+oQOK/WEgOqrSAOgsw80hvG4NRE4qhdxiqdTy+WD9Wgu70hkTRlbr5jRrG 40TVVE4dtAxzeowQAhqeKjvw6EyNQQVzXyHYWD7urlYPo3Hm0g4fjIGsmJn9O5J4 rPGX+a7e0x5TKEEArBVH3Ll51TOHEpNfqExTeOENK7TeE6VxZvjDKjuGR8EVPiUO 6YhZC5hL8voPgv2EO6ZZ0OQG4otxY47NktyV9KSjT/D7XuFusEKxl5ygzhnRTwFa 6ao1+MpgBJtJ4Js8mTZ6dA== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-transfer-encoding :content-type:content-type:date:date:feedback-id:feedback-id :from:from:in-reply-to:in-reply-to:message-id:mime-version :references:reply-to:subject:subject:to:to:x-me-proxy :x-me-sender:x-me-sender:x-sasl-enc; s=fm2; t=1744797750; x= 1744884150; bh=X0flypOkb5u31lBuemid5c4Nr1z4rinZxjq0JmdZnNI=; b=S KC183w+aZE5XMASLFdIXuadxB0uO/tubHpR5g3EFQ2GKnA3UM9cwOY87HM1qxOds tAn5XrSJKuiDEF0sGAzK+AJaasZjEIBkRVFoTG23/3o7wdgDz+06fQs+L7l0QSvh mxI4ueNv1MCg8FsiRvYT8lvS8h4YB86zM1lg1tPLJezAV397cZ8QjC3yXIu8SW1t GMGFl35vwigOAFTUcbtaeOJo/MERYz2XHAAIz50CpWcfrTzw+xHvkRPbUiIwveRJ 5T/I7EgelEsQN1Tp5k14fjcaUUnyEwKlO9oPCv6VitLQkFPvhn0gWlh5pFm7l4zy VpKdng+ojS+bhVey/7g3A== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeefvddrtddtgddvvdeitdekucetufdoteggodetrf dotffvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdggtfgfnhhsuhgsshgtrhhisggv pdfurfetoffkrfgpnffqhgenuceurghilhhouhhtmecufedttdenucesvcftvggtihhpih gvnhhtshculddquddttddmnecujfgurhephfffufggtgfgkfhfjgfvvefosehtjeertder tdejnecuhfhrohhmpefrrghtrhhitghkucfuthgvihhnhhgrrhguthcuoehpshesphhksh drihhmqeenucggtffrrghtthgvrhhnpeffueeiudejvdekheeuvdekfeffiedvueelteek udehjeetkeegvddugfdtgfeileenucevlhhushhtvghrufhiiigvpedunecurfgrrhgrmh epmhgrihhlfhhrohhmpehpshesphhkshdrihhmpdhnsggprhgtphhtthhopeekpdhmohgu vgepshhmthhpohhuthdprhgtphhtthhopehphhhilhhlihhprdifohhougduvdefsehgmh grihhlrdgtohhmpdhrtghpthhtohepthhmiiesphhosghogidrtghomhdprhgtphhtthho pehglhgruhgsihhtiiesphhhhihsihhkrdhfuhdqsggvrhhlihhnrdguvgdprhgtphhtth hopehpvghffhesphgvfhhfrdhnvghtpdhrtghpthhtohepshhtohhlvggvsehgmhgrihhl rdgtohhmpdhrtghpthhtohepghhithesvhhgvghrrdhkvghrnhgvlhdrohhrghdprhgtph htthhopehsiigvuggvrhdruggvvhesghhmrghilhdrtghomhdprhgtphhtthhopehlrdhs rdhrseifvggsrdguvg X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Wed, 16 Apr 2025 06:02:28 -0400 (EDT) Received: by vm-mail (OpenSMTPD) with ESMTPSA id 065044af (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Wed, 16 Apr 2025 10:02:27 +0000 (UTC) From: Patrick Steinhardt Date: Wed, 16 Apr 2025 12:02:14 +0200 Subject: [PATCH v3 5/7] parse-options: introduce `OPTION_UNSIGNED` Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Message-Id: <20250416-b4-pks-parse-options-integers-v3-5-d390746bea79@pks.im> References: <20250416-b4-pks-parse-options-integers-v3-0-d390746bea79@pks.im> In-Reply-To: <20250416-b4-pks-parse-options-integers-v3-0-d390746bea79@pks.im> To: git@vger.kernel.org Cc: John Paul Adrian Glaubitz , Todd Zullinger , =?utf-8?q?Ren=C3=A9_Scharfe?= , =?utf-8?q?SZEDER_G=C3=A1bor?= , Derrick Stolee , Jeff King , Phillip Wood X-Mailer: b4 0.14.2 We have two generic ways to parse integers in the "parse-options" subsystem: - `OPTION_INTEGER` parses a signed integer. - `OPTION_MAGNITUDE` parses an unsigned integer, but it also interprets suffixes like "k" or "g". Notably missing is a middle ground that parses unsigned integers without interpreting suffixes. Introduce a new `OPTION_UNSIGNED` option type to plug this gap. This option type will be used in subsequent commits. Signed-off-by: Patrick Steinhardt --- parse-options.c | 54 +++++++++++++++++++++++++++++++++++++++++++ parse-options.h | 12 ++++++++++ t/helper/test-parse-options.c | 4 +++- t/t0040-parse-options.sh | 24 ++++++++++++++++++- 4 files changed, 92 insertions(+), 2 deletions(-) diff --git a/parse-options.c b/parse-options.c index 259716efb17..e4dc22464b2 100644 --- a/parse-options.c +++ b/parse-options.c @@ -222,6 +222,60 @@ static enum parse_opt_result do_get_value(struct parse_opt_ctx_t *p, optname(opt, flags)); } } + case OPTION_UNSIGNED: + { + uintmax_t upper_bound = UINTMAX_MAX >> (bitsizeof(uintmax_t) - CHAR_BIT * opt->precision); + uintmax_t value; + + if (unset) { + value = 0; + } else if (opt->flags & PARSE_OPT_OPTARG && !p->opt) { + value = opt->defval; + } else if (get_arg(p, opt, flags, &arg)) { + return -1; + } else if (!*arg) { + return error(_("%s expects a numerical value"), + optname(opt, flags)); + } else if (*arg == '-') { + return error(_("%s does not accept negative values"), + optname(opt, flags)); + } else { + errno = 0; + value = strtoumax(arg, (char **)&s, 10); + if (*s) + return error(_("%s expects a numerical value"), + optname(opt, flags)); + if (errno == ERANGE) + return error(_("value %s for %s not in range [%"PRIuMAX",%"PRIuMAX"]"), + arg, optname(opt, flags), (uintmax_t)0, (uintmax_t)upper_bound); + if (errno) + return error_errno(_("value %s for %s cannot be parsed"), + arg, optname(opt, flags)); + + } + + if (value > upper_bound) + return error(_("value %s for %s not in range [%"PRIuMAX",%"PRIuMAX"]"), + arg, optname(opt, flags), (uintmax_t)0, (uintmax_t)upper_bound); + + switch (opt->precision) { + case 1: + *(uint8_t *)opt->value = value; + return 0; + case 2: + *(uint16_t *)opt->value = value; + return 0; + case 4: + *(uint32_t *)opt->value = value; + return 0; + case 8: + *(uint64_t *)opt->value = value; + return 0; + default: + BUG("invalid precision for option %s", + optname(opt, flags)); + } + } case OPTION_MAGNITUDE: { uintmax_t upper_bound = UINTMAX_MAX >> (bitsizeof(uintmax_t) - CHAR_BIT * opt->precision); diff --git a/parse-options.h b/parse-options.h index 55c42faa29f..aa37134dc72 100644 --- a/parse-options.h +++ b/parse-options.h @@ -25,6 +25,7 @@ enum parse_opt_type { /* options with arguments (usually) */ OPTION_STRING, OPTION_INTEGER, + OPTION_UNSIGNED, OPTION_MAGNITUDE, OPTION_CALLBACK, OPTION_LOWLEVEL_CALLBACK, @@ -224,6 +225,16 @@ struct option { .help = (h), \ .flags = (f), \ } +#define OPT_UNSIGNED_F(s, l, v, h, f) { \ + .type = OPTION_UNSIGNED, \ + .short_name = (s), \ + .long_name = (l), \ + .value = (v), \ + .precision = sizeof(*v), \ + .argh = N_("n"), \ + .help = (h), \ + .flags = (f), \ +} #define OPT_END() { \ .type = OPTION_END, \ @@ -276,6 +287,7 @@ struct option { #define OPT_CMDMODE(s, l, v, h, i) OPT_CMDMODE_F(s, l, v, h, i, 0) #define OPT_INTEGER(s, l, v, h) OPT_INTEGER_F(s, l, v, h, 0) +#define OPT_UNSIGNED(s, l, v, h) OPT_UNSIGNED_F(s, l, v, h, 0) #define OPT_MAGNITUDE(s, l, v, h) { \ .type = OPTION_MAGNITUDE, \ .short_name = (s), \ diff --git a/t/helper/test-parse-options.c b/t/helper/test-parse-options.c index 46deb4317ef..0d559288d9c 100644 --- a/t/helper/test-parse-options.c +++ b/t/helper/test-parse-options.c @@ -120,7 +120,7 @@ int cmd__parse_options(int argc, const char **argv) }; struct string_list expect = STRING_LIST_INIT_NODUP; struct string_list list = STRING_LIST_INIT_NODUP; - uint16_t m16 = 0; + uint16_t m16 = 0, u16 = 0; int16_t i16 = 0; struct option options[] = { @@ -142,6 +142,7 @@ int cmd__parse_options(int argc, const char **argv) OPT_GROUP(""), OPT_INTEGER('i', "integer", &integer, "get a integer"), OPT_INTEGER(0, "i16", &i16, "get a 16 bit integer"), + OPT_UNSIGNED(0, "u16", &u16, "get a 16 bit unsigned integer"), OPT_INTEGER('j', NULL, &integer, "get a integer, too"), OPT_MAGNITUDE('m', "magnitude", &magnitude, "get a magnitude"), OPT_MAGNITUDE(0, "m16", &m16, "get a 16 bit magnitude"), @@ -215,6 +216,7 @@ int cmd__parse_options(int argc, const char **argv) show(&expect, &ret, "boolean: %d", boolean); show(&expect, &ret, "integer: %d", integer); show(&expect, &ret, "i16: %"PRIdMAX, (intmax_t) i16); + show(&expect, &ret, "u16: %"PRIuMAX, (uintmax_t) u16); show(&expect, &ret, "magnitude: %lu", magnitude); show(&expect, &ret, "m16: %"PRIuMAX, (uintmax_t) m16); show(&expect, &ret, "timestamp: %"PRItime, timestamp); diff --git a/t/t0040-parse-options.sh b/t/t0040-parse-options.sh index 8daaf568485..66875ce0586 100755 --- a/t/t0040-parse-options.sh +++ b/t/t0040-parse-options.sh @@ -23,6 +23,7 @@ usage: test-tool parse-options -i, --[no-]integer get a integer --[no-]i16 get a 16 bit integer + --[no-]u16 get a 16 bit unsigned integer -j get a integer, too -m, --magnitude get a magnitude --m16 get a 16 bit magnitude @@ -139,6 +140,7 @@ cat >expect <<\EOF boolean: 2 integer: 1729 i16: 0 +u16: 0 magnitude: 16384 m16: 0 timestamp: 0 @@ -161,6 +163,7 @@ cat >expect <<\EOF boolean: 2 integer: 1729 i16: 9000 +u16: 5432 magnitude: 16384 m16: 32768 timestamp: 0 @@ -173,7 +176,7 @@ file: prefix/fi.le EOF test_expect_success 'long options' ' - test-tool parse-options --boolean --integer 1729 --i16 9000 --magnitude 16k \ + test-tool parse-options --boolean --integer 1729 --i16 9000 --u16 5432 --magnitude 16k \ --m16 32k --boolean --string2=321 --verbose --verbose --no-dry-run \ --abbrev=10 --file fi.le --obsolete \ >output 2>output.err && @@ -186,6 +189,7 @@ test_expect_success 'abbreviate to something longer than SHA1 length' ' boolean: 0 integer: 0 i16: 0 + u16: 0 magnitude: 0 m16: 0 timestamp: 0 @@ -262,6 +266,7 @@ cat >expect <<\EOF boolean: 1 integer: 13 i16: 0 +u16: 0 magnitude: 0 m16: 0 timestamp: 0 @@ -287,6 +292,7 @@ cat >expect <<\EOF boolean: 0 integer: 2 i16: 0 +u16: 0 magnitude: 0 m16: 0 timestamp: 0 @@ -356,6 +362,7 @@ Callback: "four", 0 boolean: 5 integer: 4 i16: 0 +u16: 0 magnitude: 0 m16: 0 timestamp: 0 @@ -383,6 +390,7 @@ cat >expect <<\EOF boolean: 1 integer: 23 i16: 0 +u16: 0 magnitude: 0 m16: 0 timestamp: 0 @@ -464,6 +472,7 @@ cat >expect <<\EOF boolean: 0 integer: 0 i16: 0 +u16: 0 magnitude: 0 m16: 0 timestamp: 0 @@ -826,4 +835,17 @@ test_expect_success 'm16 limits range' ' test_grep "value 65536 for option .m16. not in range \[0,65535\]" err ' +test_expect_success 'u16 limits range' ' + test-tool parse-options --u16 65535 >out && + test_grep "u16: 65535" out && + test_must_fail test-tool parse-options --u16 65536 2>err && + test_grep "value 65536 for option .u16. not in range \[0,65535\]" err +' + +test_expect_success 'u16 does not accept negative value' ' + test_must_fail test-tool parse-options --u16 -1 >out 2>err && + test_grep "option .u16. does not accept negative values" err && + test_must_be_empty out +' + test_done From patchwork Wed Apr 16 10:02:15 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 14053617 Received: from fhigh-a6-smtp.messagingengine.com (fhigh-a6-smtp.messagingengine.com [103.168.172.157]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id EB70A2459E9 for ; Wed, 16 Apr 2025 10:02:32 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=103.168.172.157 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1744797754; cv=none; b=n7VBHlrHwrfTE3uupxFCzkoumGJLJgDuguVJ16zLf7nH3GK0AZ07EUWFw0qC/YlWnUVYzs7s15aDTTpgKgmf+acR2D3zcTzhiQjCP2OaHdZM3yvUwjaLj4UXZeEOJu0nw7LglFMBF7ofU5BoyfJzf1EDi7JxK3vpQoR75YdraqY= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1744797754; c=relaxed/simple; bh=ZxYW7hzJk/wKOL2DK09O3p9H+7Awnxx9HfhXCX3fL5U=; h=From:Date:Subject:MIME-Version:Content-Type:Message-Id:References: In-Reply-To:To:Cc; b=g/g/X7nRofK/JLpcyvd2yQuoIrLUohV9TMwNCkWmrTp0jOkKocK7Nr4RnnIENiy+mW5F8NSwQRiGVRSigzc0Uj4fcQQRM4h+LbGLJ6xR3yolp9A2PXr+9oa3IqT+ETd9+DzMfvhGJmF/v1AGzfrigPUopJI+ezE1a/908O7CdJg= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=pks.im; spf=pass smtp.mailfrom=pks.im; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b=kY15OYKF; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b=pzvFuZq4; arc=none smtp.client-ip=103.168.172.157 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=pks.im Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=pks.im Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b="kY15OYKF"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="pzvFuZq4" Received: from phl-compute-11.internal (phl-compute-11.phl.internal [10.202.2.51]) by mailfhigh.phl.internal (Postfix) with ESMTP id E624C1140107; Wed, 16 Apr 2025 06:02:31 -0400 (EDT) Received: from phl-mailfrontend-02 ([10.202.2.163]) by phl-compute-11.internal (MEProxy); Wed, 16 Apr 2025 06:02:31 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pks.im; h=cc:cc :content-transfer-encoding:content-type:content-type:date:date :from:from:in-reply-to:in-reply-to:message-id:mime-version :references:reply-to:subject:subject:to:to; s=fm2; t=1744797751; x=1744884151; bh=G9+uXx/N3fJzi9JR6cpGxq3DsghWGV931EpWoCODjcU=; b= kY15OYKFE7WyuqCocXH6rMdCLAUUxzfmUt4FYb5BYgyW5GXEe8orqpRHV6remTrj vIFP+Xj2iMxtRyQ5zx5JEtb6FhNiCLVv/ptSZ64mL2U0Q1eJBzGOE48+hSuM9NyA EVXJt9tLo2OlGPjLBzXUPe7H2nNQb/JQRt8MZgZNt18sAyVpfLNBWp7CGFAb8WDP YHESwG1N2WZr9V915xfhfvrJjiMjPUzQMUTHT9jiw/3kzQ64mZrwD2dwmNyhOK8t amuLvow944Alm0Qalz0rHvaOjezi5qHjPtDHjQC0VcFgiR/Dw/SNHaSsq0T9YCeS 7h0hAkU09JnYHJ0K3X9p2A== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-transfer-encoding :content-type:content-type:date:date:feedback-id:feedback-id :from:from:in-reply-to:in-reply-to:message-id:mime-version :references:reply-to:subject:subject:to:to:x-me-proxy :x-me-sender:x-me-sender:x-sasl-enc; s=fm2; t=1744797751; x= 1744884151; bh=G9+uXx/N3fJzi9JR6cpGxq3DsghWGV931EpWoCODjcU=; b=p zvFuZq44N8r6mVwQbi8GCc/AciBvPJQqDq+wE8pMMOxyoxEobVwx5jCQmH2qmZ4u r2h9DwAtrkpr3YP2twBswCX9aNYyiOArCkqYUps9IwI9MCbQWQghKfqTlQIb4yMI VkOBY3X5lUNs/DhBQFT2KtNbDEyRYwWmCV9QRS5AKgG15/8+j5/vG/6EO11VQST4 TOTyfnHxap/BqfYy5Oe7qIrXIFqYaFqyzg80KpMmSDIhJenvIYW+nSPG158uCPOg fAcaygDy7LqYy2RzpQojqUFVloZ8ghNa4V3DjErXzP8Z2tVgNos87vEvTJn0LSZa tUK8a92byk5oz7vTFgHJw== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeefvddrtddtgddvvdeitdelucetufdoteggodetrf dotffvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdggtfgfnhhsuhgsshgtrhhisggv pdfurfetoffkrfgpnffqhgenuceurghilhhouhhtmecufedttdenucesvcftvggtihhpih gvnhhtshculddquddttddmnecujfgurhephfffufggtgfgkfhfjgfvvefosehtkeertder tdejnecuhfhrohhmpefrrghtrhhitghkucfuthgvihhnhhgrrhguthcuoehpshesphhksh drihhmqeenucggtffrrghtthgvrhhnpeefhfeugeelheefjeektdffhedvhfdvteefgfdt udffudevveetgeeuuedtkefhgeenucevlhhushhtvghrufhiiigvpedtnecurfgrrhgrmh epmhgrihhlfhhrohhmpehpshesphhkshdrihhmpdhnsggprhgtphhtthhopeekpdhmohgu vgepshhmthhpohhuthdprhgtphhtthhopehpvghffhesphgvfhhfrdhnvghtpdhrtghpth htohepshiivgguvghrrdguvghvsehgmhgrihhlrdgtohhmpdhrtghpthhtohepghhithes vhhgvghrrdhkvghrnhgvlhdrohhrghdprhgtphhtthhopehlrdhsrdhrseifvggsrdguvg dprhgtphhtthhopehglhgruhgsihhtiiesphhhhihsihhkrdhfuhdqsggvrhhlihhnrdgu vgdprhgtphhtthhopehtmhiisehpohgsohigrdgtohhmpdhrtghpthhtohepshhtohhlvg gvsehgmhgrihhlrdgtohhmpdhrtghpthhtohepphhhihhllhhiphdrfihoohguuddvfees ghhmrghilhdrtghomh X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Wed, 16 Apr 2025 06:02:30 -0400 (EDT) Received: by vm-mail (OpenSMTPD) with ESMTPSA id 154fcfd1 (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Wed, 16 Apr 2025 10:02:28 +0000 (UTC) From: Patrick Steinhardt Date: Wed, 16 Apr 2025 12:02:15 +0200 Subject: [PATCH v3 6/7] parse-options: detect mismatches in integer signedness Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Message-Id: <20250416-b4-pks-parse-options-integers-v3-6-d390746bea79@pks.im> References: <20250416-b4-pks-parse-options-integers-v3-0-d390746bea79@pks.im> In-Reply-To: <20250416-b4-pks-parse-options-integers-v3-0-d390746bea79@pks.im> To: git@vger.kernel.org Cc: John Paul Adrian Glaubitz , Todd Zullinger , =?utf-8?q?Ren=C3=A9_Scharfe?= , =?utf-8?q?SZEDER_G=C3=A1bor?= , Derrick Stolee , Jeff King , Phillip Wood X-Mailer: b4 0.14.2 It was reported that "t5620-backfill.sh" fails on s390x and sparc64 in a test that exercises the "--min-batch-size" command line option. The symptom was that the option didn't seem to have an effect: we didn't fetch objects with a batch size of 20, but instead fetched all objects at once. As it turns out, the root cause is that `--min-batch-size` uses `OPT_INTEGER()` to parse the command line option. While this macro expects the caller to pass a pointer to an integer, we instead pass a pointer to a `size_t`. This coincidentally works on most platforms, but it breaks apart on the mentioned platforms because they are big endian. This issue isn't specific to git-backfill(1): there are a couple of other places where we have the same type confusion going on. This indicates that the issue really is the interface that the parse-options subsystem provides -- it is simply too easy to get this wrong as there isn't any kind of compiler warning, and things just work on the most common systems. Address the systemic issue by introducing two new build asserts `BARF_UNLESS_SIGNED()` and `BARF_UNLESS_UNSIGNED()`. As the names already hint at, those macros will cause a compiler error when passed a value that is not signed or unsigned, respectively. Adapt `OPT_INTEGER()`, `OPT_UNSIGNED()` as well as `OPT_MAGNITUDE()` to use those asserts. This uncovers a small set of sites where we indeed have the same bug as in git-backfill(1). Adapt all of them to use the correct option. Reported-by: Todd Zullinger Reported-by: John Paul Adrian Glaubitz Helped-by: SZEDER Gábor Helped-by: Jeff King Signed-off-by: Patrick Steinhardt --- apply.c | 4 ++-- builtin/backfill.c | 4 ++-- builtin/column.c | 2 +- builtin/grep.c | 4 ++-- git-compat-util.h | 7 +++++++ parse-options.h | 6 +++--- 6 files changed, 17 insertions(+), 10 deletions(-) diff --git a/apply.c b/apply.c index f274a379487..a850c7d75fe 100644 --- a/apply.c +++ b/apply.c @@ -5123,8 +5123,8 @@ int apply_parse_options(int argc, const char **argv, /* Think twice before adding "--nul" synonym to this */ OPT_SET_INT('z', NULL, &state->line_termination, N_("paths are separated with NUL character"), '\0'), - OPT_INTEGER('C', NULL, &state->p_context, - N_("ensure at least lines of context match")), + OPT_UNSIGNED('C', NULL, &state->p_context, + N_("ensure at least lines of context match")), OPT_CALLBACK(0, "whitespace", state, N_("action"), N_("detect new or modified lines that have whitespace errors"), apply_option_parse_whitespace), diff --git a/builtin/backfill.c b/builtin/backfill.c index 33e1ea2f84f..d95d7a2d4d6 100644 --- a/builtin/backfill.c +++ b/builtin/backfill.c @@ -123,8 +123,8 @@ int cmd_backfill(int argc, const char **argv, const char *prefix, struct reposit .sparse = 0, }; struct option options[] = { - OPT_INTEGER(0, "min-batch-size", &ctx.min_batch_size, - N_("Minimum number of objects to request at a time")), + OPT_UNSIGNED(0, "min-batch-size", &ctx.min_batch_size, + N_("Minimum number of objects to request at a time")), OPT_BOOL(0, "sparse", &ctx.sparse, N_("Restrict the missing objects to the current sparse-checkout")), OPT_END(), diff --git a/builtin/column.c b/builtin/column.c index 50314cc2559..ce6443d5fac 100644 --- a/builtin/column.c +++ b/builtin/column.c @@ -31,7 +31,7 @@ int cmd_column(int argc, struct option options[] = { OPT_STRING(0, "command", &real_command, N_("name"), N_("lookup config vars")), OPT_COLUMN(0, "mode", &colopts, N_("layout to use")), - OPT_INTEGER(0, "raw-mode", &colopts, N_("layout to use")), + OPT_UNSIGNED(0, "raw-mode", &colopts, N_("layout to use")), OPT_INTEGER(0, "width", &copts.width, N_("maximum width")), OPT_STRING(0, "indent", &copts.indent, N_("string"), N_("padding space on left border")), OPT_STRING(0, "nl", &copts.nl, N_("string"), N_("padding space on right border")), diff --git a/builtin/grep.c b/builtin/grep.c index c4869733e1b..f23a6f1dc86 100644 --- a/builtin/grep.c +++ b/builtin/grep.c @@ -983,9 +983,9 @@ int cmd_grep(int argc, OPT_CALLBACK('C', "context", &opt, N_("n"), N_("show context lines before and after matches"), context_callback), - OPT_INTEGER('B', "before-context", &opt.pre_context, + OPT_UNSIGNED('B', "before-context", &opt.pre_context, N_("show context lines before matches")), - OPT_INTEGER('A', "after-context", &opt.post_context, + OPT_UNSIGNED('A', "after-context", &opt.post_context, N_("show context lines after matches")), OPT_INTEGER(0, "threads", &num_threads, N_("use worker threads")), diff --git a/git-compat-util.h b/git-compat-util.h index cf733b38acd..1218fcf81a4 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -110,12 +110,19 @@ DISABLE_WARNING(-Wsign-compare) # define BARF_UNLESS_COPYABLE(dst, src) \ BUILD_ASSERT_OR_ZERO(__builtin_types_compatible_p(__typeof__(*(dst)), \ __typeof__(*(src)))) + +# define BARF_UNLESS_SIGNED(var) BUILD_ASSERT_OR_ZERO(((__typeof__(var)) -1) < 0) +# define BARF_UNLESS_UNSIGNED(var) BUILD_ASSERT_OR_ZERO(((__typeof__(var)) -1) > 0) #else # define BARF_UNLESS_AN_ARRAY(arr) 0 # define BARF_UNLESS_COPYABLE(dst, src) \ BUILD_ASSERT_OR_ZERO(0 ? ((*(dst) = *(src)), 0) : \ sizeof(*(dst)) == sizeof(*(src))) + +# define BARF_UNLESS_SIGNED(var) 0 +# define BARF_UNLESS_UNSIGNED(var) 0 #endif + /* * ARRAY_SIZE - get the number of elements in a visible array * @x: the array whose size you want. diff --git a/parse-options.h b/parse-options.h index aa37134dc72..168df642386 100644 --- a/parse-options.h +++ b/parse-options.h @@ -219,7 +219,7 @@ struct option { .type = OPTION_INTEGER, \ .short_name = (s), \ .long_name = (l), \ - .value = (v), \ + .value = (v) + BARF_UNLESS_SIGNED(*(v)), \ .precision = sizeof(*v), \ .argh = N_("n"), \ .help = (h), \ @@ -229,7 +229,7 @@ struct option { .type = OPTION_UNSIGNED, \ .short_name = (s), \ .long_name = (l), \ - .value = (v), \ + .value = (v) + BARF_UNLESS_UNSIGNED(*(v)), \ .precision = sizeof(*v), \ .argh = N_("n"), \ .help = (h), \ @@ -292,7 +292,7 @@ struct option { .type = OPTION_MAGNITUDE, \ .short_name = (s), \ .long_name = (l), \ - .value = (v), \ + .value = (v) + BARF_UNLESS_UNSIGNED(*(v)), \ .precision = sizeof(*v), \ .argh = N_("n"), \ .help = (h), \ From patchwork Wed Apr 16 10:02:16 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 14053616 Received: from fhigh-a6-smtp.messagingengine.com (fhigh-a6-smtp.messagingengine.com [103.168.172.157]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id C286F2459DB for ; Wed, 16 Apr 2025 10:02:32 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=103.168.172.157 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1744797754; cv=none; b=RnFBcHJtfyzor9Tx/YC9H7gSC/ehID7bDKEZmDjKRsRiJrYGvr4jfrt/b/enasgIupE5p5ayeLiAz1JGAZmxHYJWJFxjTBb0/EnsLnqtmi0jUXiHr/QE2B9gw2fgD6gCj3+tZ47K/HTUVMJ8XnFtYKR794DmZmv9J3adyXGj0NA= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1744797754; c=relaxed/simple; bh=8BJ29en+eIXuQlQLInPtlYA/Z4dZFI33jawMnmZgkAo=; h=From:Date:Subject:MIME-Version:Content-Type:Message-Id:References: In-Reply-To:To:Cc; b=qZHllDG0vJZcoULOq5pEa1DpH2VpKP62FLFE9Z3sEEvojphU9wFgEqK1Jv3owqtjDdl5zByZQ4edUZrjnuOuY4zofC2DJQJPvsG5jsTlXC1mwimDgdHclaFDTsXzeMKz2/vpiKBqvBMfxqDXhyCrd88QtFtWxMoXFsfdiPH1o80= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=pks.im; spf=pass smtp.mailfrom=pks.im; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b=XTcBcGr/; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b=p5nCFk/T; arc=none smtp.client-ip=103.168.172.157 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=pks.im Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=pks.im Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b="XTcBcGr/"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="p5nCFk/T" Received: from phl-compute-07.internal (phl-compute-07.phl.internal [10.202.2.47]) by mailfhigh.phl.internal (Postfix) with ESMTP id 15C7D114010B; Wed, 16 Apr 2025 06:02:32 -0400 (EDT) Received: from phl-mailfrontend-01 ([10.202.2.162]) by phl-compute-07.internal (MEProxy); Wed, 16 Apr 2025 06:02:32 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pks.im; h=cc:cc :content-transfer-encoding:content-type:content-type:date:date :from:from:in-reply-to:in-reply-to:message-id:mime-version :references:reply-to:subject:subject:to:to; s=fm2; t=1744797752; x=1744884152; bh=LBRzymms4ZFDDdq69Bv3MgBVzHizF4HsdMIzB0xW6Qo=; b= XTcBcGr/ny8uhBavVylu5j+NYlyrwxTOVCod0RYDrr9f4+NgMGT++116KAeSmAE3 6Dj9lLjNQluLBmBgauqz5JFkFVxT3pgsYdbg+RWHmGd3jrOFucQ8pWqVHKzq8cs5 NYbYtWj+xwPQDhyPmPv5Exu/LwYrN0kTQ1atWS4CgdUf/XdZzvGH+YGwgCRhpEjN 5sUi272Fxkssml6S9KVcOES7rXBdVAAvPZQZgOF8a85J/ARz7F75nvC69USi4uaC Zp9emX+46XkzssaL0KpsCNC6ZXJt0OSK5K1osVIs+P+SVaZX9jddLSs59+nw3nZ7 HIkpPHd80KUG0pgf0/NeYw== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-transfer-encoding :content-type:content-type:date:date:feedback-id:feedback-id :from:from:in-reply-to:in-reply-to:message-id:mime-version :references:reply-to:subject:subject:to:to:x-me-proxy :x-me-sender:x-me-sender:x-sasl-enc; s=fm2; t=1744797752; x= 1744884152; bh=LBRzymms4ZFDDdq69Bv3MgBVzHizF4HsdMIzB0xW6Qo=; b=p 5nCFk/TeuXv4Rshj2/74jTk4l9bKEaKTiaZw/PYcZUAHYV2uveXvc3pr/URXJqgz RE+184Ouk584n7OdPYyx++OFxdTdQjqf3RzFTutJl70tQ5ErmeHiDdIwFXM/VmLj M6yI3izy+YhhJEM5ZksfTF6v400BmcfNpMLWwgI1nrQJKbhO6F99b7UN0Fs9Fg4I ooj8p16o62k6/wDtHvSoUpLxj24Vhd4P67beA3GxoRM74WZPbYy7sBWQ//R+yzGG lMEFlKnUezy5vl3kvdM1BGjvWBDDWqb8OPufRLY3FEL7f1h6uC6QwCZIehN5H+yn MxRDvZllJo/xOd3+d2wcg== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeefvddrtddtgddvvdeitdelucetufdoteggodetrf dotffvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdggtfgfnhhsuhgsshgtrhhisggv pdfurfetoffkrfgpnffqhgenuceurghilhhouhhtmecufedttdenucesvcftvggtihhpih gvnhhtshculddquddttddmnecujfgurhephfffufggtgfgkfhfjgfvvefosehtjeertder tdejnecuhfhrohhmpefrrghtrhhitghkucfuthgvihhnhhgrrhguthcuoehpshesphhksh drihhmqeenucggtffrrghtthgvrhhnpeffueeiudejvdekheeuvdekfeffiedvueelteek udehjeetkeegvddugfdtgfeileenucevlhhushhtvghrufhiiigvpedtnecurfgrrhgrmh epmhgrihhlfhhrohhmpehpshesphhkshdrihhmpdhnsggprhgtphhtthhopeekpdhmohgu vgepshhmthhpohhuthdprhgtphhtthhopehlrdhsrdhrseifvggsrdguvgdprhgtphhtth hopehsthholhgvvgesghhmrghilhdrtghomhdprhgtphhtthhopehsiigvuggvrhdruggv vhesghhmrghilhdrtghomhdprhgtphhtthhopehglhgruhgsihhtiiesphhhhihsihhkrd hfuhdqsggvrhhlihhnrdguvgdprhgtphhtthhopehpvghffhesphgvfhhfrdhnvghtpdhr tghpthhtohepghhithesvhhgvghrrdhkvghrnhgvlhdrohhrghdprhgtphhtthhopehphh hilhhlihhprdifohhougduvdefsehgmhgrihhlrdgtohhmpdhrtghpthhtohepthhmiies phhosghogidrtghomh X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Wed, 16 Apr 2025 06:02:30 -0400 (EDT) Received: by vm-mail (OpenSMTPD) with ESMTPSA id 1da16f64 (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Wed, 16 Apr 2025 10:02:29 +0000 (UTC) From: Patrick Steinhardt Date: Wed, 16 Apr 2025 12:02:16 +0200 Subject: [PATCH v3 7/7] parse-options: introduce bounded integer options Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Message-Id: <20250416-b4-pks-parse-options-integers-v3-7-d390746bea79@pks.im> References: <20250416-b4-pks-parse-options-integers-v3-0-d390746bea79@pks.im> In-Reply-To: <20250416-b4-pks-parse-options-integers-v3-0-d390746bea79@pks.im> To: git@vger.kernel.org Cc: John Paul Adrian Glaubitz , Todd Zullinger , =?utf-8?q?Ren=C3=A9_Scharfe?= , =?utf-8?q?SZEDER_G=C3=A1bor?= , Derrick Stolee , Jeff King , Phillip Wood X-Mailer: b4 0.14.2 In the preceding commits we have introduced integer precisions. The precision merely tracks bounds of the underlying data types so that we don't try to for example write a `size_t` into an `unsigned`, which could otherwise cause out-of-bounds writes. Some options may have bounds that are stricter than the underlying data type. Right now, users of any such options would have to manually verify that the value passed to such an option is inside the expected bounds. This is rather tedious, and it leads to code duplication across sites that wish to perform such bounds checks. Introduce `OPT_*_BOUNDED()` options that alleviate this issue. Users can optionally specify both a lower and upper bound, and if set we will verify that the value passed by the user is in that range. Signed-off-by: Patrick Steinhardt --- parse-options.c | 40 ++++++++++++++++++++++++++++----- parse-options.h | 52 +++++++++++++++++++++++++++++++++++++++++++ t/helper/test-parse-options.c | 5 +++++ t/t0040-parse-options.sh | 33 +++++++++++++++++++++++++++ 4 files changed, 125 insertions(+), 5 deletions(-) diff --git a/parse-options.c b/parse-options.c index e4dc22464b2..d1dffcfdf5f 100644 --- a/parse-options.c +++ b/parse-options.c @@ -177,6 +177,20 @@ static enum parse_opt_result do_get_value(struct parse_opt_ctx_t *p, intmax_t lower_bound = -upper_bound - 1; intmax_t value; + if (opt->lower_bound) { + if (opt->lower_bound < lower_bound) + BUG("invalid lower bound for option %s", optname(opt, flags)); + if (opt->lower_bound > lower_bound) + lower_bound = opt->lower_bound; + } + + if (opt->upper_bound) { + if (opt->upper_bound > (uintmax_t)upper_bound) + BUG("invalid upper bound for option %s", optname(opt, flags)); + if (opt->upper_bound < (uintmax_t)upper_bound) + upper_bound = opt->upper_bound; + } + if (unset) { value = 0; } else if (opt->flags & PARSE_OPT_OPTARG && !p->opt) { @@ -225,8 +239,16 @@ static enum parse_opt_result do_get_value(struct parse_opt_ctx_t *p, case OPTION_UNSIGNED: { uintmax_t upper_bound = UINTMAX_MAX >> (bitsizeof(uintmax_t) - CHAR_BIT * opt->precision); + uintmax_t lower_bound = 0; uintmax_t value; + if (opt->lower_bound < 0) + BUG("invalid lower bound for option %s", optname(opt, flags)); + if (opt->lower_bound > 0) + lower_bound = opt->lower_bound; + if (opt->upper_bound && opt->upper_bound < upper_bound) + upper_bound = opt->upper_bound; + if (unset) { value = 0; } else if (opt->flags & PARSE_OPT_OPTARG && !p->opt) { @@ -247,16 +269,16 @@ static enum parse_opt_result do_get_value(struct parse_opt_ctx_t *p, optname(opt, flags)); if (errno == ERANGE) return error(_("value %s for %s not in range [%"PRIuMAX",%"PRIuMAX"]"), - arg, optname(opt, flags), (uintmax_t)0, (uintmax_t)upper_bound); + arg, optname(opt, flags), (uintmax_t)lower_bound, (uintmax_t)upper_bound); if (errno) return error_errno(_("value %s for %s cannot be parsed"), arg, optname(opt, flags)); } - if (value > upper_bound) + if (value < lower_bound || value > upper_bound) return error(_("value %s for %s not in range [%"PRIuMAX",%"PRIuMAX"]"), - arg, optname(opt, flags), (uintmax_t)0, (uintmax_t)upper_bound); + arg, optname(opt, flags), (uintmax_t)lower_bound, (uintmax_t)upper_bound); switch (opt->precision) { case 1: @@ -279,8 +301,16 @@ static enum parse_opt_result do_get_value(struct parse_opt_ctx_t *p, case OPTION_MAGNITUDE: { uintmax_t upper_bound = UINTMAX_MAX >> (bitsizeof(uintmax_t) - CHAR_BIT * opt->precision); + uintmax_t lower_bound = 0; unsigned long value; + if (opt->lower_bound < 0) + BUG("invalid lower bound for option %s", optname(opt, flags)); + if (opt->lower_bound > 0) + lower_bound = opt->lower_bound; + if (opt->upper_bound && opt->upper_bound < upper_bound) + upper_bound = opt->upper_bound; + if (unset) { value = 0; } else if (opt->flags & PARSE_OPT_OPTARG && !p->opt) { @@ -293,9 +323,9 @@ static enum parse_opt_result do_get_value(struct parse_opt_ctx_t *p, optname(opt, flags)); } - if (value > upper_bound) + if (value < lower_bound || value > upper_bound) return error(_("value %s for %s not in range [%"PRIuMAX",%"PRIuMAX"]"), - arg, optname(opt, flags), (uintmax_t)0, (uintmax_t)upper_bound); + arg, optname(opt, flags), (uintmax_t)lower_bound, (uintmax_t)upper_bound); switch (opt->precision) { case 1: diff --git a/parse-options.h b/parse-options.h index 168df642386..c1ebdaf7639 100644 --- a/parse-options.h +++ b/parse-options.h @@ -97,6 +97,13 @@ typedef int parse_opt_subcommand_fn(int argc, const char **argv, * precision of the integer pointed to by `value` in number of bytes. Should * typically be its `sizeof()`. * + * `lower_bound`,`upper_bound`:: + * lower and upper bound of the integer to further restrict the accepted + * range of integer values. `0` will use the minimum and maximum values for + * the integer type of the specified precision. Specifying a bound that does + * not fit into an integer type of the specified precision will trigger a + * bug. + * * `argh`:: * token to explain the kind of argument this option wants. Does not * begin in capital letter, and does not end with a full stop. @@ -157,6 +164,8 @@ struct option { const char *long_name; void *value; size_t precision; + intmax_t lower_bound; + uintmax_t upper_bound; const char *argh; const char *help; @@ -225,6 +234,19 @@ struct option { .help = (h), \ .flags = (f), \ } +#define OPT_INTEGER_BOUNDED_F(s, l, v, lower, upper, h, f) { \ + .type = OPTION_INTEGER, \ + .short_name = (s), \ + .long_name = (l), \ + .value = (v) + BARF_UNLESS_SIGNED(*(v)), \ + .precision = sizeof(*v), \ + .lower_bound = (lower), \ + .upper_bound = (upper), \ + .argh = N_("n"), \ + .help = (h), \ + .flags = (f), \ +} + #define OPT_UNSIGNED_F(s, l, v, h, f) { \ .type = OPTION_UNSIGNED, \ .short_name = (s), \ @@ -235,6 +257,18 @@ struct option { .help = (h), \ .flags = (f), \ } +#define OPT_UNSIGNED_BOUNDED_F(s, l, v, lower, upper, h, f) { \ + .type = OPTION_UNSIGNED, \ + .short_name = (s), \ + .long_name = (l), \ + .value = (v) + BARF_UNLESS_UNSIGNED(*(v)), \ + .precision = sizeof(*v), \ + .lower_bound = (lower), \ + .upper_bound = (upper), \ + .argh = N_("n"), \ + .help = (h), \ + .flags = (f), \ +} #define OPT_END() { \ .type = OPTION_END, \ @@ -287,7 +321,12 @@ struct option { #define OPT_CMDMODE(s, l, v, h, i) OPT_CMDMODE_F(s, l, v, h, i, 0) #define OPT_INTEGER(s, l, v, h) OPT_INTEGER_F(s, l, v, h, 0) +#define OPT_INTEGER_BOUNDED(s, l, v, lower, upper, h) \ + OPT_INTEGER_BOUNDED_F(s, l, v, lower, upper, h, 0) #define OPT_UNSIGNED(s, l, v, h) OPT_UNSIGNED_F(s, l, v, h, 0) +#define OPT_UNSIGNED_BOUNDED(s, l, v, lower, upper, h) \ + OPT_UNSIGNED_BOUNDED_F(s, l, v, lower, upper, h, 0) + #define OPT_MAGNITUDE(s, l, v, h) { \ .type = OPTION_MAGNITUDE, \ .short_name = (s), \ @@ -298,6 +337,19 @@ struct option { .help = (h), \ .flags = PARSE_OPT_NONEG, \ } +#define OPT_MAGNITUDE_BOUNDED(s, l, v, lower, upper, h) { \ + .type = OPTION_MAGNITUDE, \ + .short_name = (s), \ + .long_name = (l), \ + .value = (v) + BARF_UNLESS_UNSIGNED(*(v)), \ + .precision = sizeof(*v), \ + .lower_bound = (lower), \ + .upper_bound = (upper), \ + .argh = N_("n"), \ + .help = (h), \ + .flags = PARSE_OPT_NONEG, \ +} + #define OPT_STRING(s, l, v, a, h) OPT_STRING_F(s, l, v, a, h, 0) #define OPT_STRING_LIST(s, l, v, a, h) { \ .type = OPTION_CALLBACK, \ diff --git a/t/helper/test-parse-options.c b/t/helper/test-parse-options.c index 0d559288d9c..0fcceec56a7 100644 --- a/t/helper/test-parse-options.c +++ b/t/helper/test-parse-options.c @@ -120,7 +120,9 @@ int cmd__parse_options(int argc, const char **argv) }; struct string_list expect = STRING_LIST_INIT_NODUP; struct string_list list = STRING_LIST_INIT_NODUP; + uint32_t mbounded = 0, ubounded = 0; uint16_t m16 = 0, u16 = 0; + int32_t ibounded = 0; int16_t i16 = 0; struct option options[] = { @@ -142,10 +144,13 @@ int cmd__parse_options(int argc, const char **argv) OPT_GROUP(""), OPT_INTEGER('i', "integer", &integer, "get a integer"), OPT_INTEGER(0, "i16", &i16, "get a 16 bit integer"), + OPT_INTEGER_BOUNDED(0, "ibounded", &ibounded, -10, 10, "get a bounded integer between [-10,10]"), OPT_UNSIGNED(0, "u16", &u16, "get a 16 bit unsigned integer"), + OPT_UNSIGNED_BOUNDED(0, "ubounded", &ubounded, 10, 100, "get a bounded unsigned integer between [10,100]"), OPT_INTEGER('j', NULL, &integer, "get a integer, too"), OPT_MAGNITUDE('m', "magnitude", &magnitude, "get a magnitude"), OPT_MAGNITUDE(0, "m16", &m16, "get a 16 bit magnitude"), + OPT_MAGNITUDE_BOUNDED(0, "mbounded", &mbounded, 10, 100, "get a bounded magnitude between [10,100]"), OPT_SET_INT(0, "set23", &integer, "set integer to 23", 23), OPT_CMDMODE(0, "mode1", &integer, "set integer to 1 (cmdmode option)", 1), OPT_CMDMODE(0, "mode2", &integer, "set integer to 2 (cmdmode option)", 2), diff --git a/t/t0040-parse-options.sh b/t/t0040-parse-options.sh index 66875ce0586..d76165c2053 100755 --- a/t/t0040-parse-options.sh +++ b/t/t0040-parse-options.sh @@ -23,10 +23,13 @@ usage: test-tool parse-options -i, --[no-]integer get a integer --[no-]i16 get a 16 bit integer + --[no-]ibounded get a bounded integer between [-10,10] --[no-]u16 get a 16 bit unsigned integer + --[no-]ubounded get a bounded unsigned integer between [10,100] -j get a integer, too -m, --magnitude get a magnitude --m16 get a 16 bit magnitude + --mbounded get a bounded magnitude between [10,100] --[no-]set23 set integer to 23 --mode1 set integer to 1 (cmdmode option) --mode2 set integer to 2 (cmdmode option) @@ -848,4 +851,34 @@ test_expect_success 'u16 does not accept negative value' ' test_must_be_empty out ' +test_expect_success 'ibounded does not accept outside range' ' + test_must_fail test-tool parse-options --ibounded -11 >out 2>err && + test_grep "value -11 for option .ibounded. not in range \[-10,10\]" err && + test_must_fail test-tool parse-options --ibounded 11 >out 2>err && + test_grep "value 11 for option .ibounded. not in range \[-10,10\]" err && + test-tool parse-options --ibounded -10 && + test-tool parse-options --ibounded 0 && + test-tool parse-options --ibounded 10 +' + +test_expect_success 'ubounded does not accept outside range' ' + test_must_fail test-tool parse-options --ubounded 9 >out 2>err && + test_grep "value 9 for option .ubounded. not in range \[10,100\]" err && + test_must_fail test-tool parse-options --ubounded 101 >out 2>err && + test_grep "value 101 for option .ubounded. not in range \[10,100\]" err && + test-tool parse-options --ubounded 10 && + test-tool parse-options --ubounded 50 && + test-tool parse-options --ubounded 100 +' + +test_expect_success 'mbounded does not accept outside range' ' + test_must_fail test-tool parse-options --mbounded 9 >out 2>err && + test_grep "value 9 for option .mbounded. not in range \[10,100\]" err && + test_must_fail test-tool parse-options --mbounded 101 >out 2>err && + test_grep "value 101 for option .mbounded. not in range \[10,100\]" err && + test-tool parse-options --mbounded 10 && + test-tool parse-options --mbounded 50 && + test-tool parse-options --mbounded 100 +' + test_done