Re: [PATCH 06/20] builtin/receive-pack: convert portions to struct object_id

From: Duy Nguyen <pclouds@gmail.com>
Date: Mar 20, 2017, 12:56 UTC
Message-ID: <CACsJy8Ch0=ovmmfHikQG04K249TEpA3HONWpknUn_kWbQa=8Fw@mail.gmail.com>
In-Reply-To: <20170318211954.564030-7-sandals@crustytoothpaste.net>

On Sun, Mar 19, 2017 at 4:19 AM, brian m. carlson <sandals@crustytoothpaste.net> wrote:

Show 17 quoted lines

> @@ -1489,23 +1489,24 @@ static struct command **queue_command(struct command **tail,
>                                       const char *line,
>                                       int linelen)
>  {
> -       unsigned char old_sha1[20], new_sha1[20];
> +       struct object_id old_oid, new_oid;
>         struct command *cmd;
>         const char *refname;
>         int reflen;
> +       const char *p;
>
> -       if (linelen < 83 ||
> -           line[40] != ' ' ||
> -           line[81] != ' ' ||
> -           get_sha1_hex(line, old_sha1) ||
> -           get_sha1_hex(line + 41, new_sha1))
> +       if (!linelen ||

I think you can skip this. The old code needed "< 83" because of the random accesses to [40] and [81] but you don't do that anymore. parse_oid_hex() can handle empty hex strings fine.

> +           parse_oid_hex(line, &old_oid, &p) ||
> +           *p++ != ' ' ||
> +           parse_oid_hex(p, &new_oid, &p) ||
> +           *p++ != ' ')

maybe "|| *p)" as well? I think the old code, with "linelen < 83", makes sure reflen is at least one. Not sure what FLEX_ALLOC_MEM would do if reflen is zero.

Show 14 quoted lines

>                 die("protocol error: expected old/new/ref, got '%s'", line);
>
> -       refname = line + 82;
> -       reflen = linelen - 82;
> +       refname = p;
> +       reflen = linelen - (p - line);
>         FLEX_ALLOC_MEM(cmd, ref_name, refname, reflen);
> -       hashcpy(cmd->old_sha1, old_sha1);
> -       hashcpy(cmd->new_sha1, new_sha1);
> +       oidcpy(&cmd->old_oid, &old_oid);
> +       oidcpy(&cmd->new_oid, &new_oid);
>         *tail = cmd;
>         return &cmd->next;
>  }

-- 
Duy