Re: [PATCH v2 2/8] object.c: add a utility function for "expected type X, got Y"
- From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
- Date
- Apr 22, 2021, 06:10 UTC
- Message-ID
- <877dku3l53.fsf@evledraar.gmail.com>
- In-Reply-To
- <20210421220300.1022458-1-jonathantanmy@google.com>
On Thu, Apr 22 2021, Jonathan Tan wrote:
Show 25 quoted lines
>> diff --git a/merge-recursive.c b/merge-recursive.c
>> index 7618303f7b..b952106203 100644
>> --- a/merge-recursive.c
>> +++ b/merge-recursive.c
>> @@ -2999,6 +2999,7 @@ static int read_oid_strbuf(struct merge_options *opt,
>> if (!buf)
>> return err(opt, _("cannot read object %s"), oid_to_hex(oid));
>> if (type != OBJ_BLOB) {
>> + const char* msg = oid_is_type_or_die_msg(oid, OBJ_BLOB, &type);
>> free(buf);
>> return err(opt, _("object %s is not a blob"), oid_to_hex(oid));
>> }
>
> Stray extra line.
>
>> +void oid_is_type_or_die(const struct object_id *oid,
>> + enum object_type want,
>> + enum object_type *type)
>> +{
>
> Thanks - this looks like a good simplification.
>
> Why is type a pointer? Maybe it's to better distinguish the values at
> the call site (one pointer, one not), but this solution is confusing
> too.Yeah I came up with it because of that, so you wouldn't confuse the OBJ_COMMIT with (presumably) a variable with the same.
But in some other cases I end up having to do:
enum object_type type = OBJ_COMMIT;
And then pass that &type in, do you think it's worth it? Maybe I should just change it...
Show 6 quoted lines
>> +int oid_is_type_or_error(const struct object_id *oid,
>> + enum object_type want,
>> + enum object_type *type)
>> +{
>
> Same comment.