Hi Johannes
Thanks for cleaning this up - I'm not sure why I didn't just write
something like this in the first place.
Best Wishes
Phillip
On 15/05/2025 14:11, Johannes Schindelin via GitGitGadget wrote:
From: Johannes Schindelin <johannes.schindelin@xxxxxx>
In 3e81bccdf3 (sequencer: factor out todo command name parsing,
2019-06-27), a `return` statement was introduced that basically was a
long sequence of conditions, combined with `&&`, except for the last
condition which is not really a condition but an assignment.
The point of this construct was to return 1 (i.e. `true`) from the
function if all of those conditions held true, and also assign the `bol`
pointer to the end of the parsed command.
Some static analyzers are really unhappy about such constructs. And
human readers are at least puzzled, if not confused, by seeing a single
`=` inside a chain of conditions where they would have expected to see
`==` instead and, based on experience, immediately suspect a typo.
Let's help all of this by turning this into the more verbose, more
readable form of an `if` construct that both assigns the pointer as well
as returns 1 if all of the conditions hold true.
Signed-off-by: Johannes Schindelin <johannes.schindelin@xxxxxx>
---
sequencer.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index b5c4043757e9..e5e3bc6fa5ea 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -2600,9 +2600,12 @@ static int is_command(enum todo_command command, const char **bol)
const char nick = todo_command_info[command].c;
const char *p = *bol;
- return (skip_prefix(p, str, &p) || (nick && *p++ == nick)) &&
- (*p == ' ' || *p == '\t' || *p == '\n' || *p == '\r' || !*p) &&
- (*bol = p);
+ if ((skip_prefix(p, str, &p) || (nick && *p++ == nick)) &&
+ (*p == ' ' || *p == '\t' || *p == '\n' || *p == '\r' || !*p)) {
+ *bol = p;
+ return 1;
+ }
+ return 0;
}
static int check_label_or_ref_arg(enum todo_command command, const char *arg)
