Re: [PATCH v3 1/2] Factor find_pack_entry()'s core out
- From
Nicolas Pitre <nico@fluxnic.net>
- Date
- Feb 1, 2012, 15:59 UTC
- Message-ID
- <alpine.LFD.2.02.1202011056140.2759@xanadu.home>
- In-Reply-To
- <1328104135-475-1-git-send-email-pclouds@gmail.com>
On Wed, 1 Feb 2012, Nguyễn Thái Ngọc Duy wrote:
Show 16 quoted lines
> > Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> > --- > sha1_file.c | 59 +++++++++++++++++++++++++++++++++-------------------------- > 1 files changed, 33 insertions(+), 26 deletions(-) > > diff --git a/sha1_file.c b/sha1_file.c > index 88f2151..ff5bf42 100644 > --- a/sha1_file.c > +++ b/sha1_file.c > @@ -2010,11 +2010,42 @@ int is_pack_valid(struct packed_git *p) > return !open_packed_git(p); > } > > +static int find_pack_entry_1(const unsigned char *sha1, > + struct packed_git *p, struct pack_entry *e)
This looks all goot but the name. Pretty please, try to find something that is more descriptive than "1". Suggestions: "find_pack_entry_lookup", "find_pack_entry_inner", etc.
With that fixed, you can add:
Acked-by: Nicolas Pitre <nico@fluxnic.net>
Show 78 quoted lines
> +{
> + off_t offset;
> + if (p->num_bad_objects) {
> + unsigned i;
> + for (i = 0; i < p->num_bad_objects; i++)
> + if (!hashcmp(sha1, p->bad_object_sha1 + 20 * i))
> + return 0;
> + }
> +
> + offset = find_pack_entry_one(sha1, p);
> + if (!offset)
> + return 0;
> +
> + /*
> + * We are about to tell the caller where they can locate the
> + * requested object. We better make sure the packfile is
> + * still here and can be accessed before supplying that
> + * answer, as it may have been deleted since the index was
> + * loaded!
> + */
> + if (!is_pack_valid(p)) {
> + warning("packfile %s cannot be accessed", p->pack_name);
> + return 0;
> + }
> + e->offset = offset;
> + e->p = p;
> + hashcpy(e->sha1, sha1);
> + return 1;
> +}
> +
> static int find_pack_entry(const unsigned char *sha1, struct pack_entry *e)
> {
> static struct packed_git *last_found = (void *)1;
> struct packed_git *p;
> - off_t offset;
>
> prepare_packed_git();
> if (!packed_git)
> @@ -2022,35 +2053,11 @@ static int find_pack_entry(const unsigned char *sha1, struct pack_entry *e)
> p = (last_found == (void *)1) ? packed_git : last_found;
>
> do {
> - if (p->num_bad_objects) {
> - unsigned i;
> - for (i = 0; i < p->num_bad_objects; i++)
> - if (!hashcmp(sha1, p->bad_object_sha1 + 20 * i))
> - goto next;
> - }
> -
> - offset = find_pack_entry_one(sha1, p);
> - if (offset) {
> - /*
> - * We are about to tell the caller where they can
> - * locate the requested object. We better make
> - * sure the packfile is still here and can be
> - * accessed before supplying that answer, as
> - * it may have been deleted since the index
> - * was loaded!
> - */
> - if (!is_pack_valid(p)) {
> - warning("packfile %s cannot be accessed", p->pack_name);
> - goto next;
> - }
> - e->offset = offset;
> - e->p = p;
> - hashcpy(e->sha1, sha1);
> + if (find_pack_entry_1(sha1, p, e)) {
> last_found = p;
> return 1;
> }
>
> - next:
> if (p == last_found)
> p = packed_git;
> else
> --
> 1.7.8.36.g69ee2
>