From 791aeddfa2fdb9e830e24c50c97bb5e8bf3613e6 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Thu, 17 Apr 2025 12:49:42 +0200 Subject: parse-options: detect mismatches in integer signedness MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Signed-off-by: Junio C Hamano --- builtin/column.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'builtin/column.c') diff --git a/builtin/column.c b/builtin/column.c index 50314cc255..ce6443d5fa 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")), -- cgit v1.2.3