mingw: avoid the comma operator

[dscho]

I wonder how many more times I will deal with the comma operator...

cc: Jeff King peff@peff.net

[dscho]

/submit

[gitgitgadget]

Submitted as pull.2007.git.1763412374866.gitgitgadget@gmail.com

To fetch this version into FETCH_HEAD:

git fetch https://github.com/gitgitgadget/git/ pr-2007/dscho/mingw-avoid-the-comma-operator-5660--v1

To fetch this version to local tag pr-2007/dscho/mingw-avoid-the-comma-operator-5660--v1:

git fetch --no-tags https://github.com/gitgitgadget/git/ tag pr-2007/dscho/mingw-avoid-the-comma-operator-5660--v1

[gitgitgadget]

On the Git mailing list, Junio C Hamano wrote (reply to this):

"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
writes:

> From: Johannes Schindelin <johannes.schindelin@gmx.de>
>
> The pattern `return errno = ..., -1;` is observed several times in
> `compat/mingw.c`. It has served us well over the years, but now clang
> starts complaining:
>
>   compat/mingw.c:723:24: error: possible misuse of comma operator here [-Werror,-Wcomma]
>     723 |                 return errno = ENOSYS, -1;
>         |                                      ^
>
> See for example this failing workflow run:
> https://github.com/git-for-windows/git-sdk-arm64/actions/runs/15457893907/job/43513458823#step:8:201
>
> Let's appease clang (and also reduce the use of the no longer common
> comma operator).
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
>     mingw: avoid the comma operator
>     
>     I wonder how many more times I will deal with the comma operator...

;-)

>  	/* only these flags are supported */
> -	if ((oflags & ~O_CREAT) != (O_WRONLY | O_APPEND))
> -		return errno = ENOSYS, -1;
> +	if ((oflags & ~O_CREAT) != (O_WRONLY | O_APPEND)) {
> +		errno = ENOSYS;
> +		return -1;
> +	}

Good riddance.  It indeed is somewhat hard to read, especially
because it may not be apparent to readers how "A = B, C" binds
(answer: B gets assigned to A and then the whole thing yields C).

I wonder if

	return (errno = ENOSYS), -1;

is accepted by the compiler, but in these error handling we do not
have to be cute, and updated code that is both simple and stupid
reads very well.

Will queue.  Thanks.

[gitgitgadget]

This patch series was integrated into seen via git@06ac4b0.

[gitgitgadget]

On the Git mailing list, Jeff King wrote (reply to this):

On Mon, Nov 17, 2025 at 02:16:34PM -0800, Junio C Hamano wrote:

> >  	/* only these flags are supported */
> > -	if ((oflags & ~O_CREAT) != (O_WRONLY | O_APPEND))
> > -		return errno = ENOSYS, -1;
> > +	if ((oflags & ~O_CREAT) != (O_WRONLY | O_APPEND)) {
> > +		errno = ENOSYS;
> > +		return -1;
> > +	}
> 
> Good riddance.  It indeed is somewhat hard to read, especially
> because it may not be apparent to readers how "A = B, C" binds
> (answer: B gets assigned to A and then the whole thing yields C).
> 
> I wonder if
> 
> 	return (errno = ENOSYS), -1;
> 
> is accepted by the compiler, but in these error handling we do not
> have to be cute, and updated code that is both simple and stupid
> reads very well.

Agreed that we are best avoiding comma operators when we can. There is
one spot where we use it, though, and I haven't figured out a good way
around it: in the error() macro wrapper.

I guess it does not cause the same compiler complaints because there is
no assignment in it. So if nobody is complaining, we can just avert our
eyes when looking at the macro. :)

-Peff

[gitgitgadget]

User Jeff King <peff@peff.net> has been added to the cc: list.

[gitgitgadget]

This patch series was integrated into seen via git@41a27af.

[gitgitgadget]

This branch is now known as js/mingw-assign-comma-fix.

[gitgitgadget]

This patch series was integrated into seen via git@51e3f26.

[gitgitgadget]

This patch series was integrated into seen via git@29bebc7.

[gitgitgadget]

This patch series was integrated into next via git@d1a5807.

[gitgitgadget]

There was a status update in the "New Topics" section about the branch js/mingw-assign-comma-fix on the Git mailing list:

The "return errno = EFOO, -1" construct, which is heavily used in
compat/mingw.c and triggers warnings under "-Wcomma", has been
rewritten to avoid the warnings.

Will merge to 'master'.
source: <pull.2007.git.1763412374866.gitgitgadget@gmail.com>

[gitgitgadget]

This patch series was integrated into seen via git@81651aa.

[gitgitgadget]

This patch series was integrated into seen via git@fe05518.