From: Johannes Schindelin Date: Thu, 15 May 2025 13:11:49 +0000 (+0000) Subject: sequencer: stop pretending that an assignment is a condition X-Git-Tag: v2.50.0-rc0~7^2 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=22488332393646cfa4263bcb24836f492876406e;p=thirdparty%2Fgit.git sequencer: stop pretending that an assignment is a condition 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 Signed-off-by: Junio C Hamano --- diff --git a/sequencer.c b/sequencer.c index b5c4043757..e5e3bc6fa5 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)