On Wed, Jun 18, 2014 at 3:49 PM, Jeff King <peff@xxxxxxxx> wrote:
> Fast-import does a lot of parsing of commands and
> dispatching to sub-functions. For example, given "option
> foo", we might recognize "option " using starts_with, and
> then hand it off to parse_option() to do the rest.
>
> However, we do not let parse_option know that we have parsed
> the first part already. It gets the full buffer, and has to
> skip past the uninteresting bits. Some functions simply add
> a magic constant:
>
> char *option = command_buf.buf + 7;
>
> Others use strlen:
>
> char *option = command_buf.buf + strlen("option ");
>
> And others use strchr:
>
> char *option = strchr(command_buf.buf, ' ') + 1;
>
> All of these are brittle and easy to get wrong (especially
> given that the starts_with call and the code that assumes
> the presence of the prefix are far apart). Instead, we can
> use skip_prefix, and just pass each handler a pointer to its
> arguments.
>
> Signed-off-by: Jeff King <peff@xxxxxxxx>
> ---
> diff --git a/fast-import.c b/fast-import.c
> index a3ffe30..5f17adb 100644
> --- a/fast-import.c
> +++ b/fast-import.c
> @@ -2713,20 +2706,22 @@ static void parse_new_commit(void)
>
> /* file_change* */
> while (command_buf.len > 0) {
> - if (starts_with(command_buf.buf, "M "))
> - file_change_m(b);
> - else if (starts_with(command_buf.buf, "D "))
> - file_change_d(b);
> - else if (starts_with(command_buf.buf, "R "))
> - file_change_cr(b, 1);
> - else if (starts_with(command_buf.buf, "C "))
> - file_change_cr(b, 0);
> - else if (starts_with(command_buf.buf, "N "))
> - note_change_n(b, &prev_fanout);
> + const char *v;
This declaration of 'v' shadows the 'v' added by patch 8/16 earlier in
the function.
> + if (skip_prefix(command_buf.buf, "M ", &v))
> + file_change_m(v, b);
> + else if (skip_prefix(command_buf.buf, "D ", &v))
> + file_change_d(v, b);
> + else if (skip_prefix(command_buf.buf, "R ", &v))
> + file_change_cr(v, b, 1);
> + else if (skip_prefix(command_buf.buf, "C ", &v))
> + file_change_cr(v, b, 0);
> + else if (skip_prefix(command_buf.buf, "N ", &v))
> + note_change_n(v, b, &prev_fanout);
> else if (!strcmp("deleteall", command_buf.buf))
> file_change_deleteall(b);
> - else if (starts_with(command_buf.buf, "ls "))
> - parse_ls(b);
> + else if (skip_prefix(command_buf.buf, "ls ", &v))
> + parse_ls(v, b);
> else {
> unread_command_buf = 1;
> break;
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html