Re: [PATCH 1/2] avoid shifting signed integers 31 bits

From: Junio C Hamano <gitster@pobox.com>
Date: Dec 30, 2015, 00:09 UTC
Message-ID: <xmqq4mf0g5la.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <20151229063545.GA30340@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

Show 13 quoted lines

> diff --git a/diff.h b/diff.h
> index f7208ad..893f446 100644
> --- a/diff.h
> +++ b/diff.h
> @@ -91,7 +91,7 @@ typedef struct strbuf *(*diff_prefix_fn_t)(struct diff_options *opt, void *data)
>  #define DIFF_OPT_DIRSTAT_BY_LINE     (1 << 28)
>  #define DIFF_OPT_FUNCCONTEXT         (1 << 29)
>  #define DIFF_OPT_PICKAXE_IGNORE_CASE (1 << 30)
> -#define DIFF_OPT_DEFAULT_FOLLOW_RENAMES (1 << 31)
> +#define DIFF_OPT_DEFAULT_FOLLOW_RENAMES (1U << 31)
>  
>  #define DIFF_OPT_TST(opts, flag)    ((opts)->flags & DIFF_OPT_##flag)
>  #define DIFF_OPT_TOUCHED(opts, flag)    ((opts)->touched_flags & DIFF_OPT_##flag)

Thanks.

Seeing (1 << 30) and (1U <<31) together made me feel that we are way _too_ explicit being careful about 32-bit archs (iow, it would be more consistent to turn all of these "1 <<" into "1U <<"), but at the same time, (1 << 30) won't be broken unless we are on 31-bit arch in the sense that if we are on 30-bit or smaller arch the expression is already broken with or without "U", and if we are on 32-bit or more, then with or without "U" we are OK---which made me feel somewhat funny.

In any case, these two are good changes.  Thanks.