Re: [PATCH 12/16] sha1_file: add for_each iterators for loose and packed objects

[Ramsay Jones <ramsay@ramsay1.demon.co.uk>]

On 05/10/14 09:15, René Scharfe wrote:
> Am 03.10.2014 um 22:32 schrieb Jeff King:
>> We typically iterate over the reachable objects in a
>> repository by starting at the tips and walking the graph.
>> There's no easy way to iterate over all of the objects,
>> including unreachable ones. Let's provide a way of doing so.
>>
>> Signed-off-by: Jeff King <peff@peff.net>
>> ---
>>   cache.h     | 11 +++++++++++
>>   sha1_file.c | 62 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>   2 files changed, 73 insertions(+)
>>
>> diff --git a/cache.h b/cache.h
>> index 7abe7f6..3826b4b 100644
>> --- a/cache.h
>> +++ b/cache.h
>> @@ -1270,6 +1270,17 @@ int for_each_loose_file_in_objdir(const char *path,
>>                     each_loose_subdir_fn subdir_cb,
>>                     void *data);
>>
>> +/*
>> + * Iterate over loose and packed objects in both the local
>> + * repository and any alternates repositories.
>> + */
>> +typedef int each_packed_object_fn(const unsigned char *sha1,
>> +                  struct packed_git *pack,
>> +                  uint32_t pos,
>> +                  void *data);
>> +extern int for_each_loose_object(each_loose_object_fn, void *);
>> +extern int for_each_packed_object(each_packed_object_fn, void *);
>> +
>>   struct object_info {
>>       /* Request */
>>       enum object_type *typep;
>> diff --git a/sha1_file.c b/sha1_file.c
>> index 9fdad47..d017289 100644
>> --- a/sha1_file.c
>> +++ b/sha1_file.c
>> @@ -3313,3 +3313,65 @@ int for_each_loose_file_in_objdir(const char *path,
>>       strbuf_release(&buf);
>>       return r;
>>   }
>> +
>> +struct loose_alt_odb_data {
>> +    each_loose_object_fn *cb;
>> +    void *data;
>> +};
>> +
>> +static int loose_from_alt_odb(struct alternate_object_database *alt,
>> +                  void *vdata)
>> +{
>> +    struct loose_alt_odb_data *data = vdata;
>> +    return for_each_loose_file_in_objdir(alt->base,
>> +                         data->cb, NULL, NULL,
>> +                         data->data);
>> +}
>> +
>> +int for_each_loose_object(each_loose_object_fn cb, void *data)
>> +{
>> +    struct loose_alt_odb_data alt;
>> +    int r;
>> +
>> +    r = for_each_loose_file_in_objdir(get_object_directory(),
>> +                      cb, NULL, NULL, data);
>> +    if (r)
>> +        return r;
>> +
>> +    alt.cb = cb;
>> +    alt.data = data;
>> +    return foreach_alt_odb(loose_from_alt_odb, &alt);
>> +}
>> +
>> +int for_each_object_in_pack(struct packed_git *p, each_packed_object_fn cb, void *data)
> 
> Should this one be declared static?  It seems to be used only in sha1_file.c.

Heh, I was just about to make the same observation myself (with included patch).

I could imagine this function being useful elsewhere, but until it gains some
more external callers I think it should remain static (so it doesn't cause a
sparse warning), rather than add an extern declaration to cache.h (which would
also suppress sparse).

ATB,
Ramsay Jones

> 
>> +{
>> +    uint32_t i;
>> +    int r = 0;
>> +
>> +    for (i = 0; i < p->num_objects; i++) {
>> +        const unsigned char *sha1 = nth_packed_object_sha1(p, i);
>> +
>> +        if (!sha1)
>> +            return error("unable to get sha1 of object %u in %s",
>> +                     i, p->pack_name);
>> +
>> +        r = cb(sha1, p, i, data);
>> +        if (r)
>> +            break;
>> +    }
>> +    return r;
>> +}
>> +
>> +int for_each_packed_object(each_packed_object_fn cb, void *data)
>> +{
>> +    struct packed_git *p;
>> +    int r = 0;
>> +
>> +    prepare_packed_git();
>> +    for (p = packed_git; p; p = p->next) {
>> +        r = for_each_object_in_pack(p, cb, data);
>> +        if (r)
>> +            break;
>> +    }
>> +    return 0;
>> +}
> 
> Perhaps return r instead here?
> 
> René
> 
> 
> -- 
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> .
>