Re: [PATCH 2/3] sequencer: factor out todo command name parsing
- From
René Scharfe <l.s.r@web.de>
- Date
- Jun 25, 2019, 17:03 UTC
- Message-ID
- <ebaa5956-37aa-29b6-60c7-f287e06db505@web.de>
- In-Reply-To
- <ebe8a2b3a1638e8457b1b703b8c4890a9b8904a0.1561457483.git.gitgitgadget@gmail.com>
Am 25.06.19 um 12:11 schrieb Phillip Wood via GitGitGadget:
Show 5 quoted lines
> From: Phillip Wood <phillip.wood@dunelm.org.uk> > > Factor out the code that parses the name of the command at the start of > each line in the todo file into it's own function so that it can be used > in the next commit.
"Factor out" sounds like functionality is intended to not be changed...
Show 23 quoted lines
>
> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
> ---
> sequencer.c | 19 +++++++++++++------
> 1 file changed, 13 insertions(+), 6 deletions(-)
>
> diff --git a/sequencer.c b/sequencer.c
> index 919e3153f5..793f86bf9a 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -2076,6 +2076,18 @@ const char *todo_item_get_arg(struct todo_list *todo_list,
> return todo_list->buf.buf + item->arg_offset;
> }
>
> +static int is_command(enum todo_command command, const char **bol)
> +{
> + const char *str = todo_command_info[command].str;
> + const char nick = todo_command_info[command].c;
> + const char *p = *bol + 1;
> +
> + return skip_prefix(*bol, str, bol) ||
> + ((nick && **bol == nick) &&
> + (*p == ' ' || *p == '\t' || *p == '\n' || *p == '\r' || !*p) &&^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ ... but this adds support for LF, CR and NUL as separators after short commands...
Show 14 quoted lines
> + (*bol = p));
> +}
> +
> static int parse_insn_line(struct repository *r, struct todo_item *item,
> const char *buf, const char *bol, char *eol)
> {
> @@ -2097,12 +2109,7 @@ static int parse_insn_line(struct repository *r, struct todo_item *item,
> }
>
> for (i = 0; i < TODO_COMMENT; i++)
> - if (skip_prefix(bol, todo_command_info[i].str, &bol)) {
> - item->command = i;
> - break;
> - } else if ((bol + 1 == eol || bol[1] == ' ' || bol[1] == '\t') &&^^^^^^^^^^^^^^ ... while this removes the check against the string's length.
Is this safe? It probably (hopefully?) is, as skip_prefix() requires bol to point to a NUL-terminated string already.
But is_command() could do the old (and shorter) eol check just as well, so the next question is: Why this change?
Show 7 quoted lines
> - *bol == todo_command_info[i].c) {
> - bol++;
> + if (is_command(i, &bol)) {
> item->command = i;
> break;
> }
>