Re: [PATCH v2 06/10] diff-delta: explicitly mark intentional use of the comma operator

[Patrick Steinhardt <ps@pks.im>]

On Tue, Mar 25, 2025 at 11:32:10PM +0000, Johannes Schindelin via GitGitGadget wrote:
> From: Johannes Schindelin <johannes.schindelin@gmx.de>
> 
> The comma operator is a somewhat obscure C feature that is often used by
> mistake and can even cause unintentional code flow. That is why the
> `-Wcomma` option of clang was introduced: To identify unintentional uses
> of the comma operator.
> 
> Intentional uses include situations where one wants to avoid curly
> brackets around multiple statements that need to be guarded by a
> condition. This is the case here, as the repetitive nature of the
> statements is easier to see for a human reader this way.
> 
> To mark this usage as intentional, the return value of the statement
> before the comma needs to be cast to `void`, which we do here.
> 
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
>  diff-delta.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/diff-delta.c b/diff-delta.c
> index a4faf73829b..a03ba10b2be 100644
> --- a/diff-delta.c
> +++ b/diff-delta.c
> @@ -439,18 +439,18 @@ create_delta(const struct delta_index *index,
>  			i = 0x80;
>  
>  			if (moff & 0x000000ff)
> -				out[outpos++] = moff >> 0,  i |= 0x01;
> +				(void)(out[outpos++] = moff >> 0),  i |= 0x01;
>  			if (moff & 0x0000ff00)
> -				out[outpos++] = moff >> 8,  i |= 0x02;
> +				(void)(out[outpos++] = moff >> 8),  i |= 0x02;
>  			if (moff & 0x00ff0000)
> -				out[outpos++] = moff >> 16, i |= 0x04;
> +				(void)(out[outpos++] = moff >> 16), i |= 0x04;
>  			if (moff & 0xff000000)
> -				out[outpos++] = moff >> 24, i |= 0x08;
> +				(void)(out[outpos++] = moff >> 24), i |= 0x08;
>  
>  			if (msize & 0x00ff)
> -				out[outpos++] = msize >> 0, i |= 0x10;
> +				(void)(out[outpos++] = msize >> 0), i |= 0x10;
>  			if (msize & 0xff00)
> -				out[outpos++] = msize >> 8, i |= 0x20;
> +				(void)(out[outpos++] = msize >> 8), i |= 0x20;

Hm. I think the end result is even more confusing than before. Why don't
we introduce curly braces here, same as in preceding commits?

Patrick