# [BUG]: backfill min batch size test case failure on s390x 3 messages from 2025-04-22 to 2025-04-22. Participants: Pranav P, Patrick Steinhardt. Thread: https://gitlist.dev/t/63328 ## Pranav P, 2025-04-22 12:31 Subject: [BUG]: backfill min batch size test case failure on s390x Message-ID: URL: https://gitlist.dev/e/BY5PR15MB35396BEBCAB2C559080F08C6A0BB2%40BY5PR15MB3539.namprd15.prod.outlook.com ``` Hi, When running `make test` on an s390x machine in Debian it is failing on 'do partial clone 2, backfill min batch size' Reference: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1102106 After processing the command line arguments structure member min_batch_size should have had the value 20 Instead of having the value 20 (--min-batch-size=20) it was having a very large value min_batch_size in `struct backfill_context` is of type `size_t` and since in the function cmd_backfill, in the options struct it is passed on to OPT_INTEGER, which eventually causes ˋˋˋ *(int *)opt->value = strtol(arg, (char **)&s, 10); ˋˋˋ in parse-options.c line 188. This is writing the data in the first 4 bytes of min_batch_size and on big endian systems this will lead min_batch_size to be a big number. This issue is immediately visible in little endian systems. Changing OPT_INTEGER to OPT_MAGNITUDE seems to be working on x86 and s390x ˋˋˋ diff --git a/builtin/backfill.c b/builtin/backfill.c index 18f9701487..33e1ea2f84 100644 --- a/builtin/backfill.c +++ b/builtin/backfill.c @@ -123,7 +123,7 @@ int cmd_backfill(int argc, const char **argv, const char *prefix, struct reposit .sparse = 0, }; struct option options[] = { - OPT_MAGNITUDE(0, "min-batch-size", &ctx.min_batch_size, + OPT_INTEGER(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")), ˋˋˋ But on systems where size_t which not be unsigned long, this might lead to an issue. So, one other suggestion I have is to change the data type of min_batch_size from size_t to int. But I am not able to determine whether a practical upper bound for min_batch_size would exceed what an int variable can store. With that clarification, I can a raise patch for the issue. Please review the rest of the bug report below. [System Info] git version: git version 2.49.0.391.g4bbb303af6 cpu: s390x built from commit: 4bbb303af69990ccd05fe3a2eb58a1ce036f8220 sizeof-long: 8 sizeof-size_t: 8 shell-path: /bin/sh libcurl: 8.13.0 OpenSSL: OpenSSL 3.4.1 11 Feb 2025 zlib: 1.3.1 SHA-1: SHA1_DC SHA-256: SHA256_BLK uname: Linux 6.1.0-31-s390x #1 SMP Debian 6.1.128-1 (2025-02-07) s390x compiler info: gnuc: 14.2 libc info: glibc: 2.41 $SHELL (typically, interactive shell): /bin/bash [Enabled Hooks] I am fairly new to opensource and was following the `git bugreport`. So I am extremely sorry for any lack of clarity in the report. Thanks, Pranav ``` ## Pranav P, 2025-04-22 12:35 Subject: Re: [BUG]: backfill min batch size test case failure on s390x Message-ID: URL: https://gitlist.dev/e/BY5PR15MB35398BF51EAFC520421163AFA0BB2%40BY5PR15MB3539.namprd15.prod.outlook.com In-Reply-To: ``` Hi, Sorry I had pasted the wrong git diff. The corrected one is mentioned below. diff --git a/builtin/backfill.c b/builtin/backfill.c index 33e1ea2f84..18f9701487 100644 --- a/builtin/backfill.c +++ b/builtin/backfill.c @@ -123,7 +123,7 @@ 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, + OPT_MAGNITUDE(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")), -- Thanks, Pranav ________________________________________ From: Pranav P Sent: Tuesday, April 22, 2025 6:01 PM To: git@vger.kernel.org Subject: [BUG]: backfill min batch size test case failure on s390x Hi, When running `make test` on an s390x machine in Debian it is failing on 'do partial clone 2, backfill min batch size' Reference: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1102106 After processing the command line arguments structure member min_batch_size should have had the value 20 Instead of having the value 20 (--min-batch-size=20) it was having a very large value min_batch_size in `struct backfill_context` is of type `size_t` and since in the function cmd_backfill, in the options struct it is passed on to OPT_INTEGER, which eventually causes ˋˋˋ *(int *)opt->value = strtol(arg, (char **)&s, 10); ˋˋˋ in parse-options.c line 188. This is writing the data in the first 4 bytes of min_batch_size and on big endian systems this will lead min_batch_size to be a big number. This issue is immediately visible in little endian systems. Changing OPT_INTEGER to OPT_MAGNITUDE seems to be working on x86 and s390x ˋˋˋ diff --git a/builtin/backfill.c b/builtin/backfill.c index 18f9701487..33e1ea2f84 100644 --- a/builtin/backfill.c +++ b/builtin/backfill.c @@ -123,7 +123,7 @@ int cmd_backfill(int argc, const char **argv, const char *prefix, struct reposit .sparse = 0, }; struct option options[] = { - OPT_MAGNITUDE(0, "min-batch-size", &ctx.min_batch_size, + OPT_INTEGER(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")), ˋˋˋ But on systems where size_t which not be unsigned long, this might lead to an issue. So, one other suggestion I have is to change the data type of min_batch_size from size_t to int. But I am not able to determine whether a practical upper bound for min_batch_size would exceed what an int variable can store. With that clarification, I can a raise patch for the issue. Please review the rest of the bug report below. [System Info] git version: git version 2.49.0.391.g4bbb303af6 cpu: s390x built from commit: 4bbb303af69990ccd05fe3a2eb58a1ce036f8220 sizeof-long: 8 sizeof-size_t: 8 shell-path: /bin/sh libcurl: 8.13.0 OpenSSL: OpenSSL 3.4.1 11 Feb 2025 zlib: 1.3.1 SHA-1: SHA1_DC SHA-256: SHA256_BLK uname: Linux 6.1.0-31-s390x #1 SMP Debian 6.1.128-1 (2025-02-07) s390x compiler info: gnuc: 14.2 libc info: glibc: 2.41 $SHELL (typically, interactive shell): /bin/bash [Enabled Hooks] I am fairly new to opensource and was following the `git bugreport`. So I am extremely sorry for any lack of clarity in the report. Thanks, Pranav ``` ## Patrick Steinhardt, 2025-04-22 13:53 Subject: Re: [BUG]: backfill min batch size test case failure on s390x Message-ID: URL: https://gitlist.dev/e/aAefbU-MmY_734oB%40pks.im In-Reply-To: ``` Hi, On Tue, Apr 22, 2025 at 12:31:18PM +0000, Pranav P wrote: > Hi, > > When running `make test` on an s390x machine in Debian it is failing on 'do partial clone 2, backfill min batch size' > Reference: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1102106 > > After processing the command line arguments structure member min_batch_size should have had the value 20 > > Instead of having the value 20 (--min-batch-size=20) it was having a very large value > > min_batch_size in `struct backfill_context` is of type `size_t` and since in the function cmd_backfill, in the > options struct it is passed on to OPT_INTEGER, which eventually causes > > ˋˋˋ > *(int *)opt->value = strtol(arg, (char **)&s, 10); > ˋˋˋ > in parse-options.c line 188. This is writing the data in the first 4 bytes of min_batch_size and on big endian > systems this will lead min_batch_size to be a big number. This issue is immediately visible in little endian systems. > > Changing OPT_INTEGER to OPT_MAGNITUDE seems to be working on x86 and s390x Yup, indeed. > ˋˋˋ > diff --git a/builtin/backfill.c b/builtin/backfill.c > index 18f9701487..33e1ea2f84 100644 > --- a/builtin/backfill.c > +++ b/builtin/backfill.c > @@ -123,7 +123,7 @@ int cmd_backfill(int argc, const char **argv, const char *prefix, struct reposit > .sparse = 0, > }; > struct option options[] = { > - OPT_MAGNITUDE(0, "min-batch-size", &ctx.min_batch_size, > + OPT_INTEGER(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")), > ˋˋˋ > > But on systems where size_t which not be unsigned long, this might lead to an issue. > So, one other suggestion I have is to change the data type of min_batch_size from size_t to int. But I am not able to > determine whether a practical upper bound for min_batch_size would exceed what an int variable can store. > With that clarification, I can a raise patch for the issue. True, as well. Overall the current state is a bit unfortunate because it's so easy to get this wrong, and the compiler won't even spit out a warning. > I am fairly new to opensource and was following the `git bugreport`. So I am extremely sorry for any lack of clarity in the report. No need to be sorry, this bug report contains all the details we'd need and is of quite high quality :) The only thing is that we've already got this bug reported via [1]. The underlying issue is addressed via [2], which has already been merged to `next`. It also fixes the underlying issue you observed with different integer widths as well as with signedness. Thanks! Patrick [1]: https://lore.kernel.org/git/89257ab82cd60d135cce02d51eacee7ec35c1c37.camel@physik.fu-berlin.de/ [2]: https://lore.kernel.org/git/20250401-b4-pks-parse-options-integers-v1-0-a628ad40c3b4@pks.im/ ```