[v2,04/10] clar: avoid using the comma operator unnecessarily

Commit Message

Johannes Schindelin March 25, 2025, 11:32 p.m. UTC

From: Johannes Schindelin <[e-mail]>

The comma operator is a somewhat obscure C feature that is often used by
mistake and can even cause unintentional code flow. In this instance, it
makes the code harder to read than necessary, too. Better use a
semicolon instead.

Signed-off-by: Johannes Schindelin <[e-mail]>
---
 t/unit-tests/clar/clar/fs.h | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

Comments

Patrick Steinhardt March 26, 2025, 5:54 a.m. UTC | #1

On Tue, Mar 25, 2025 at 11:32:08PM +0000, Johannes Schindelin via GitGitGadget wrote:
> From: Johannes Schindelin <[e-mail]>
> 
> The comma operator is a somewhat obscure C feature that is often used by
> mistake and can even cause unintentional code flow. In this instance, it
> makes the code harder to read than necessary, too. Better use a
> semicolon instead.

This code has changed upstream already, but let's roll with your change
anyway. I plan to update the clar to the upstream version soonish once I
have landed integer comparisons, and will take care that there aren't
any other comment operators left.

Patrick

Johannes Schindelin March 26, 2025, 7:03 a.m. UTC | #2

Hi Patrick,

On Wed, 26 Mar 2025, Patrick Steinhardt wrote:

> On Tue, Mar 25, 2025 at 11:32:08PM +0000, Johannes Schindelin via GitGitGadget wrote:
> > From: Johannes Schindelin <[e-mail]>
> >
> > The comma operator is a somewhat obscure C feature that is often used by
> > mistake and can even cause unintentional code flow. In this instance, it
> > makes the code harder to read than necessary, too. Better use a
> > semicolon instead.
>
> This code has changed upstream already, but let's roll with your change
> anyway. I plan to update the clar to the upstream version soonish once I
> have landed integer comparisons, and will take care that there aren't
> any other comment operators left.

Thank you for putting so much energy into improving the tests, I really
appreciate it!

Ciao,
Johannes

Patch

diff --git a/t/unit-tests/clar/clar/fs.h b/t/unit-tests/clar/clar/fs.h
index 8b206179fc4..2203743fb48 100644
--- a/t/unit-tests/clar/clar/fs.h
+++ b/t/unit-tests/clar/clar/fs.h
@@ -376,9 +376,12 @@  fs_copydir_helper(const char *source, const char *dest, int dest_mode)
 	mkdir(dest, dest_mode);
 
 	cl_assert_(source_dir = opendir(source), "Could not open source dir");
-	while ((d = (errno = 0, readdir(source_dir))) != NULL) {
+	for (;;) {
 		char *child;
 
+		errno = 0;
+		if ((d = readdir(source_dir)) == NULL)
+			break;
 		if (!strcmp(d->d_name, ".") || !strcmp(d->d_name, ".."))
 			continue;
 
@@ -479,9 +482,12 @@  fs_rmdir_helper(const char *path)
 	struct dirent *d;
 
 	cl_assert_(dir = opendir(path), "Could not open dir");
-	while ((d = (errno = 0, readdir(dir))) != NULL) {
+	for (;;) {
 		char *child;
 
+		errno = 0;
+		if ((d = readdir(dir)) == NULL)
+			break;
 		if (!strcmp(d->d_name, ".") || !strcmp(d->d_name, ".."))
 			continue;