All of lore.kernel.org
 help / color / mirror / Atom feed
From: "SZEDER Gábor" <szeder.dev@gmail.com>
To: Patrick Steinhardt <ps@pks.im>
Cc: Todd Zullinger <tmz@pobox.com>,
	John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>,
	git <git@vger.kernel.org>,
	Derrick Stolee <derrickstolee@github.com>
Subject: Re: Testsuite failure on s390x and sparc64 after 6840fe9ee2
Date: Mon, 31 Mar 2025 20:17:57 +0200	[thread overview]
Message-ID: <Z+rcVY7KqEuF1wFw@szeder.dev> (raw)
In-Reply-To: <Z-qKGqpbdaW9WCrP@pks.im>

On Mon, Mar 31, 2025 at 02:27:06PM +0200, Patrick Steinhardt wrote:
> One thing I stumbled over: the `--min-batch-size` parameter is parsed
> using `OPT_INTEGER()`, which expects the value pointer to point to an
> integer. But we pass `struct backfill_context::min_batch_size`, which is
> of type `size_t`. Maybe that's causing us to end up with an invalid
> value?

We could teach parse-options to verify at compile time that it got a
'value' pointer to an appropriately sized variable with a simple
trick:

diff --git a/parse-options.h b/parse-options.h
index 997ffbee80..ac63f9548a 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -213,7 +213,7 @@ struct option {
 	.type = OPTION_INTEGER, \
 	.short_name = (s), \
 	.long_name = (l), \
-	.value = (v), \
+	.value = (v) + 0/(sizeof(*(v)) == sizeof(int)), \
 	.argh = N_("n"), \
 	.help = (h), \
 	.flags = (f), \

This bug would then cause a compiler error like this:

      CC builtin/backfill.o
  In file included from builtin/backfill.c:7:
  builtin/backfill.c: In function ‘cmd_backfill’:
  ./parse-options.h:216:25: error: division by zero [-Werror=div-by-zero]
    216 |         .value = (v) + 0/(sizeof(*v) == sizeof(int)), \
        |                         ^
  ./parse-options.h:272:37: note: in expansion of macro ‘OPT_INTEGER_F’
    272 | #define OPT_INTEGER(s, l, v, h)     OPT_INTEGER_F(s, l, v, h, 0)
        |                                     ^~~~~~~~~~~~~
  builtin/backfill.c:126:17: note: in expansion of macro ‘OPT_INTEGER’
    126 |                 OPT_INTEGER(0, "min-batch-size", &ctx.min_batch_size,
        |                 ^~~~~~~~~~~
  cc1: all warnings being treated as errors
  make: *** [Makefile:2811: builtin/backfill.o] Error 1

Alas, the change is ugly (and we should do the same for many other
OPT_* macros as well) and the error message is far from
to-the-point...  Turning this into something usable would require a
more clever trick, and that's more than I can devote to this issue.


  parent reply	other threads:[~2025-03-31 18:18 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-03-26 20:42 Testsuite failure on s390x and sparc64 after 6840fe9ee2 John Paul Adrian Glaubitz
2025-03-26 22:27 ` Todd Zullinger
2025-03-28  9:29   ` Patrick Steinhardt
2025-03-28  9:30     ` Patrick Steinhardt
2025-03-28  9:38     ` John Paul Adrian Glaubitz
2025-03-28 14:08       ` Todd Zullinger
2025-03-28 15:37         ` Todd Zullinger
2025-03-31 12:27           ` Patrick Steinhardt
2025-03-31 15:48             ` Todd Zullinger
2025-03-31 18:17             ` SZEDER Gábor [this message]
2025-04-01  2:33               ` Jeff King
2025-04-01  3:10                 ` Jeff King
2025-04-01 11:43                   ` Patrick Steinhardt
2025-04-01 15:04                     ` Patrick Steinhardt

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Z+rcVY7KqEuF1wFw@szeder.dev \
    --to=szeder.dev@gmail.com \
    --cc=derrickstolee@github.com \
    --cc=git@vger.kernel.org \
    --cc=glaubitz@physik.fu-berlin.de \
    --cc=ps@pks.im \
    --cc=tmz@pobox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.