Re: [PATCH v3 14/14] ref-filter: parse objects on demand

[Junio C Hamano <gitster@pobox.com>]

Patrick Steinhardt <ps@pks.im> writes:

> +static int get_or_parse_object(struct expand_data *data, const char *refname,
> +			       struct object **object, struct strbuf *err, int *eaten)
> +{
> +	if (!data->maybe_object) {
> +		data->maybe_object = parse_object_buffer(the_repository, &data->oid, data->type,
> +							 data->size, data->content, eaten);
> +		if (!data->maybe_object)
> +			return strbuf_addf_ret(err, -1, _("parse_object_buffer failed on %s for %s"),
> +					       oid_to_hex(&data->oid), refname);
> +	}
> +
> +	*object = data->maybe_object;
> +	return 0;
> +}

I wonder if the calling convention for this function can use further
ergonomic improvements, after seeing that ...

> +		if (!tag) {
> +			struct object *object;
> +
> +			ret = get_or_parse_object(data, refname, &object, err, eaten);
> +			if (ret < 0)
> +				return ret;
> +
> +			tag = (struct tag *) object;
> +		}

... this pattern is repeated three times in the patch.  If the
helper cannot legitimately return 0 (success) while stuffing NULL to
the object, then something like this

		if (!tag) {
			tag = (struct tag *) get_or_parse(data,	refname, err, eaten);
			if (!tag)
				return -1;
		}

ought to be sufficient for this caller and the other two.  I also
wonder if this can further be simplified, but stopping at the above
is fine to me.

In any case, the overall idea of this step is very much in line with
the original vision of ref-filter.c:populate_value() to try avoiding
unnecessary work as much as possible, and it is very much welcome.

Nice performance improvements.

Will queue.  Thanks.