Re: [PATCH 13/18] fill_sha1_file: write "boring" characters

From: Jacob Keller <jacob.keller@gmail.com>
Date: Oct 4, 2016, 06:13 UTC
Message-ID: <CA+P7+xpOxoRBDZGF_CU1Q-SYiQZtMx2vuwQKS0og864awZod5g@mail.gmail.com>
In-Reply-To: <20161003203555.6xadycotmmkuf34h@sigill.intra.peff.net>

On Mon, Oct 3, 2016 at 1:35 PM, Jeff King <peff@peff.net> wrote:

Show 10 quoted lines

> This function forms a sha1 as "xx/yyyy...", but skips over
> the slot for the slash rather than writing it, leaving it to
> the caller to do so. It also does not bother to put in a
> trailing NUL, even though every caller would want it (we're
> forming a path which by definition is not a directory, so
> the only thing to do with it is feed it to a system call).
>
> Let's make the lives of our callers easier by just writing
> out the internal "/" and the NUL.
>

Ya this makes sense.

Show 22 quoted lines

> Signed-off-by: Jeff King <peff@peff.net>
> ---
>  sha1_file.c | 12 +++++-------
>  1 file changed, 5 insertions(+), 7 deletions(-)
>
> diff --git a/sha1_file.c b/sha1_file.c
> index 70c3e2f..c6308c1 100644
> --- a/sha1_file.c
> +++ b/sha1_file.c
> @@ -178,10 +178,12 @@ static void fill_sha1_path(char *pathbuf, const unsigned char *sha1)
>         for (i = 0; i < 20; i++) {
>                 static char hex[] = "0123456789abcdef";
>                 unsigned int val = sha1[i];
> -               char *pos = pathbuf + i*2 + (i > 0);
> -               *pos++ = hex[val >> 4];
> -               *pos = hex[val & 0xf];
> +               *pathbuf++ = hex[val >> 4];
> +               *pathbuf++ = hex[val & 0xf];
> +               if (!i)
> +                       *pathbuf++ = '/';
>         }
> +       *pathbuf = '\0';

I think this makes a lot more sense than making the callers have to do this.

Thanks, Jake

Show 24 quoted lines

>  }
>
>  const char *sha1_file_name(const unsigned char *sha1)
> @@ -198,8 +200,6 @@ const char *sha1_file_name(const unsigned char *sha1)
>                 die("insanely long object directory %s", objdir);
>         memcpy(buf, objdir, len);
>         buf[len] = '/';
> -       buf[len+3] = '/';
> -       buf[len+42] = '\0';
>         fill_sha1_path(buf + len + 1, sha1);
>         return buf;
>  }
> @@ -406,8 +406,6 @@ struct alternate_object_database *alloc_alt_odb(const char *dir)
>
>         ent->name = ent->scratch + dirlen + 1;
>         ent->scratch[dirlen] = '/';
> -       ent->scratch[dirlen + 3] = '/';
> -       ent->scratch[entlen-1] = 0;
>
>         return ent;
>  }
> --
> 2.10.0.618.g82cc264
>