From: Patrick Steinhardt Date: Wed, 26 Mar 2025 05:54:40 GMT Subject: Re: [PATCH v2 06/10] diff-delta: explicitly mark intentional use of the comma operator Message-ID: In-Reply-To: <5e0e8325620395196608a0851f1f6fc9408f6090.1742945534.git.gitgitgadget@gmail.com> On Tue, Mar 25, 2025 at 11:32:10PM +0000, Johannes Schindelin via GitGitGadget wrote: > From: Johannes Schindelin > > 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 > --- > 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