Re: [PATCH] test-progress: fix test failures on big-endian systems

From: Jeff King <peff@peff.net>
Date: Oct 24, 2019, 17:55 UTC
Message-ID: <20191024175549.GA12892@sigill.intra.peff.net>
In-Reply-To: <20191024172442.GM4348@szeder.dev>

On Thu, Oct 24, 2019 at 07:24:42PM +0200, SZEDER Gábor wrote:

Show 7 quoted lines

> On Mon, Oct 21, 2019 at 09:52:11AM +0900, Junio C Hamano wrote:
> > I can sympathize, but I do not think it is worth inventing OPT_U64()
> > or adding "int total_i" whose value is assigned to "u64 total" after
> > parsing a command line arg with OPT_INTEGER() into the former.
> 
> I agree, we should wait for the first real use case where specifying a
> larger-than-32bit integer actually makes sense in practice.

Anybody who wants to refer to a file or memory size would want that. We have a few cases in pack-objects already, but they use OPT_MAGNITUDE() instead. Which makes sense. Anything we expect to be that gigantic would want to have the shorthand to say "4G" or whatever.

(As a side note, I notice that OPT_MAGNITUDE uses "unsigned long", which probably needs to be adjusted for Windows. Dealing with all of the void-pointer type-punning fallout from that will be fun. :) ).

Show 6 quoted lines

> It's output looks like this when applied to an older version without
> the big-endian fix upthread:
> 
>   potential error at apply.c:4982:26:
>     passing variable 'state -> p_context' of type 'unsigned int' to OPT_INTEGER
>     OPT_INTEGER expects an int

Looks like this found a lot of the same issues my "intp" conversion found. My recollection is that mine found more, but it might just have been that the compiler's warning output is rather verbose. TBH, I didn't carefully catalog them; as soon as I saw the problem, I went to bed in disgust. ;)

Show 20 quoted lines

> diff --git a/contrib/coccinelle/parse-options.cocci b/contrib/coccinelle/parse-options.cocci
> new file mode 100644
> index 0000000000..e0cddef421
> --- /dev/null
> +++ b/contrib/coccinelle/parse-options.cocci
> @@ -0,0 +1,18 @@
> +@ optint @
> +identifier opts;
> +type T;
> +T var;
> +expression SHORT, LONG, HELP;
> +position p;
> +@@
> +struct option opts[] = { ..., OPT_INTEGER(SHORT, LONG, &var@p, HELP), ...};
> +
> +@ script:python @
> +p << optint.p;
> +var << optint.var;
> +vartype << optint.T;
> +@@

I avoided relying on the python interpreter for previous cocci patches, since some builds don't have it (IIRC, the one in Debian experimental doesn't, but that one has not graduated even to "unstable" after several years, so maybe it's not worth worrying about).

If we do start using python, we might want to revisit 4d168e742a (coccinelle: use <...> for function exclusion, 2018-08-28), which describes a faster python alternative in the commit message.

-Peff