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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

> 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. :) ).

> 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. ;)

> 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



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux