diff mbox series

[v3,07/10] wildmatch: avoid using of the comma operator

Message ID 1d0ce59cb684c2878f1a0db9daeae23ef4abb763.1743076383.git.gitgitgadget@gmail.com (mailing list archive)
State New
Headers show
Series Avoid the comma operator | expand

Commit Message

Johannes Schindelin March 27, 2025, 11:53 a.m. UTC
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.

In this instance, the usage is intentional because it allows storing the
value of the current character as `prev_ch` before making the next
character the current one, all of which happens in the loop condition
that lets the loop stop at a closing bracket.

However, it is hard to read.

The chosen alternative to using the comma operator is to move those
assignments from the condition into the loop body; In this particular
case that requires special care because the loop body contains a
`continue` for the case where a character class is found that starts
with `[:` but does not end in `:]` (and the assignments should occur
even when that code path is taken), which needs to be turned into a
`goto`.

Helped-by: Phillip Wood <phillip.wood@dunelm.org.uk>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 wildmatch.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)
diff mbox series

Patch

diff --git a/wildmatch.c b/wildmatch.c
index 8ea29141bd7..69a2ae7000d 100644
--- a/wildmatch.c
+++ b/wildmatch.c
@@ -223,7 +223,7 @@  static int dowild(const uchar *p, const uchar *text, unsigned int flags)
 						p_ch = '[';
 						if (t_ch == p_ch)
 							matched = 1;
-						continue;
+						goto next;
 					}
 					if (CC_EQ(s,i, "alnum")) {
 						if (ISALNUM(t_ch))
@@ -268,7 +268,10 @@  static int dowild(const uchar *p, const uchar *text, unsigned int flags)
 					p_ch = 0; /* This makes "prev_ch" get set to 0. */
 				} else if (t_ch == p_ch)
 					matched = 1;
-			} while (prev_ch = p_ch, (p_ch = *++p) != ']');
+next:
+				prev_ch = p_ch;
+				p_ch = *++p;
+			} while (p_ch != ']');
 			if (matched == negated ||
 			    ((flags & WM_PATHNAME) && t_ch == '/'))
 				return WM_NOMATCH;