git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH 11/11] sequencer: stop pretending that an assignment is a condition

From
PWPhillip Wood <phillip.wood123@gmail.com>
Date
May 16, 2025, 10:13 UTC
Message-ID
<8641d540-516f-4dd2-bafa-cbc2568b5281@crinan.ddns.net>
In-Reply-To
<7a54005bd26ac17cb6d99a2e18932f97575d4aca.1747314709.git.gitgitgadget@gmail.com>
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:
Show 45 quoted lines
> From: Johannes Schindelin <johannes.schindelin@gmx.de>
> 
> 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@gmx.de>
> ---
>   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)
Previous: Jeff KingNext: Johannes Schindelin via GitGitGadget
Message 30 of 36 in “CodeQL-inspired fixes”
  1. 00/11 CodeQL-inspired fixesJohannes Schindelin via GitGitGadget, May 15, 2025
  2. 02/11 fetch: carefully clear local variable's address after useJohannes Schindelin via GitGitGadget, May 15, 2025
  3. Jeff KingMay 15, 2025
  4. 03/11 commit-graph: avoid malloc'ing a local variableJohannes Schindelin via GitGitGadget, May 15, 2025
  5. Jeff KingMay 15, 2025
  6. Junio C HamanoMay 15, 2025
  7. 04/11 upload-pack: rename `enum` to reflect the operationJohannes Schindelin via GitGitGadget, May 15, 2025
  8. Jeff KingMay 15, 2025
  9. 05/11 has_dir_name(): make code more obviousJohannes Schindelin via GitGitGadget, May 15, 2025
  10. Jeff KingMay 15, 2025
  11. 06/11 fetch: avoid unnecessary work when there is no current branchJohannes Schindelin via GitGitGadget, May 15, 2025
  12. Jeff KingMay 15, 2025
  13. 07/11 Avoid redundant conditionsJohannes Schindelin via GitGitGadget, May 15, 2025
  14. Jeff KingMay 15, 2025
  15. 08/11 trace2: avoid "futile conditional"Johannes Schindelin via GitGitGadget, May 15, 2025
  16. Jeff KingMay 15, 2025
  17. 09/11 commit-graph: avoid using stale stack addressesJohannes Schindelin via GitGitGadget, May 15, 2025
  18. Jeff KingMay 15, 2025
  19. 10/11 bundle-uri: avoid using undefined output of `sscanf()`Johannes Schindelin via GitGitGadget, May 15, 2025
  20. Junio C HamanoMay 15, 2025
  21. Jeff KingMay 15, 2025
  22. Phillip WoodMay 16, 2025
  23. Phillip WoodMay 16, 2025
  24. Jeff KingMay 16, 2025
  25. Phillip WoodMay 19, 2025
  26. Jeff KingMay 22, 2025
  27. 11/11 sequencer: stop pretending that an assignment is a conditionJohannes Schindelin via GitGitGadget, May 15, 2025
  28. Junio C HamanoMay 15, 2025
  29. Jeff KingMay 15, 2025
  30. Phillip WoodMay 16, 2025
  31. 01/11 commit: simplify codeJohannes Schindelin via GitGitGadget, May 15, 2025
  32. Jeff KingMay 15, 2025
  33. Junio C HamanoMay 15, 2025
  34. Jeff KingMay 15, 2025
  35. Jeff KingMay 15, 2025
  36. Junio C HamanoMay 15, 2025

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.