Re: [PATCH v2 4/4] ref-filter: use oid_object_info() to get object

From: Оля Тележная <olyatelezhnaya@gmail.com>
Date: Jul 17, 2018, 07:44 UTC
Message-ID: <CAL21BmnDqv9yib5Vqh4q7zj1hd_bmhO+xnDVJBi-zM0Cqtsevw@mail.gmail.com>
In-Reply-To: <xmqqr2k2sy02.fsf@gitster-ct.c.googlers.com>

2018-07-16 23:53 GMT+03:00 Junio C Hamano <gitster@pobox.com>:

Show 29 quoted lines

> Olga Telezhnaya <olyatelezhnaya@gmail.com> writes:
>
>> -static int get_object(struct ref_array_item *ref, const struct object_id *oid,
>> -                   int deref, struct object **obj, struct strbuf *err)
>> +static int get_object(struct ref_array_item *ref, int deref, struct object **obj,
>> +                   struct expand_data *oi, struct strbuf *err)
>>  {
>> -     int eaten;
>> -     int ret = 0;
>> -     unsigned long size;
>> -     enum object_type type;
>> -     void *buf = read_object_file(oid, &type, &size);
>> -     if (!buf)
>> -             ret = strbuf_addf_ret(err, -1, _("missing object %s for %s"),
>> -                                   oid_to_hex(oid), ref->refname);
>> -     else {
>> -             *obj = parse_object_buffer(oid, type, size, buf, &eaten);
>> -             if (!*obj)
>> -                     ret = strbuf_addf_ret(err, -1, _("parse_object_buffer failed on %s for %s"),
>> -                                           oid_to_hex(oid), ref->refname);
>> -             else
>> -                     grab_values(ref->value, deref, *obj, buf, size);
>> +     /* parse_object_buffer() will set eaten to 0 if free() will be needed */
>> +     int eaten = 1;
>
> Hmph, doesn't this belong to the previous step?  In other words,
> isn't the result of applying 1-3/4 has a bug that can leave eaten
> uninitialized (and base decision to free(buf) later on it), and
> isn't this change a fix for it?

Oh. I was thinking that it was new bug created by me. Now I see that previously we had the same problem. I guess it's better to make separate commit to fix it. Hope I can do it in this patch (I don't want to create separate patch because of so minor update). Thank you! I will fix it soon.

Show 30 quoted lines

>
>> +     if (oi->info.contentp) {
>> +             /* We need to know that to use parse_object_buffer properly */
>> +             oi->info.sizep = &oi->size;
>> +             oi->info.typep = &oi->type;
>>       }
>> +     if (oid_object_info_extended(the_repository, &oi->oid, &oi->info,
>> +                                  OBJECT_INFO_LOOKUP_REPLACE))
>> +             return strbuf_addf_ret(err, -1, _("missing object %s for %s"),
>> +                                    oid_to_hex(&oi->oid), ref->refname);
>> +
>> +     if (oi->info.contentp) {
>> +             *obj = parse_object_buffer(&oi->oid, oi->type, oi->size, oi->content, &eaten);
>> +             if (!obj) {
>> +                     if (!eaten)
>> +                             free(oi->content);
>> +                     return strbuf_addf_ret(err, -1, _("parse_object_buffer failed on %s for %s"),
>> +                                            oid_to_hex(&oi->oid), ref->refname);
>> +             }
>> +             grab_values(ref->value, deref, *obj, oi->content, oi->size);
>> +     }
>> +
>> +     grab_common_values(ref->value, deref, oi);
>>       if (!eaten)
>> -             free(buf);
>> -     return ret;
>> +             free(oi->content);
>> +     return 0;
>>  }
>