On Sat, Jun 12 2021, ZheNing Hu via GitGitGadget wrote:
> From: ZheNing Hu <adlternative@xxxxxxxxx>
>
> Let `populate_value()`, `get_ref_atom_value()` and
> `format_ref_array_item()` get the return value of `get_value()`
> correctly. This can help us later let `cat-file --batch` get the
> correct error message and return value of `get_value()`.
>
> Mentored-by: Christian Couder <christian.couder@xxxxxxxxx>
> Mentored-by: Hariom Verma <hariom18599@xxxxxxxxx>
> Signed-off-by: ZheNing Hu <adlternative@xxxxxxxxx>
> ---
> ref-filter.c | 19 +++++++++++--------
> 1 file changed, 11 insertions(+), 8 deletions(-)
>
> diff --git a/ref-filter.c b/ref-filter.c
> index 8868cf98f090..420c0bf9384f 100644
> --- a/ref-filter.c
> +++ b/ref-filter.c
> @@ -1808,7 +1808,7 @@ static char *get_worktree_path(const struct used_atom *atom, const struct ref_ar
> static int populate_value(struct ref_array_item *ref, struct strbuf *err)
> {
> struct object *obj;
> - int i;
> + int i, ret = 0;
The usual style is to not bunch up variables based on type, but only if
they're related, i.e. we'd do:
if i, j; /* proceed to use i and j in two for-loops */
But:
int i; /* for the for-loop */
int ret = 0; /* for our return value */
(Without the comments)
> struct object_info empty = OBJECT_INFO_INIT;
>
> CALLOC_ARRAY(ref->value, used_atom_cnt);
> @@ -1965,8 +1965,8 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err)
>
>
> oi.oid = ref->objectname;
> - if (get_object(ref, 0, &obj, &oi, err))
> - return -1;
> + if ((ret = get_object(ref, 0, &obj, &oi, err)))
> + return ret;
Maybe more personal style, I'd just write this as:
ret = x();
if (!ret)
return ret;
Makes it easier to read and balance parens in your head for the common
case...
> @@ -2585,10 +2588,10 @@ int format_ref_array_item(struct ref_array_item *info,
> if (cp < sp)
> append_literal(cp, sp, &state);
> pos = parse_ref_filter_atom(format, sp + 2, ep, error_buf);
> - if (pos < 0 || get_ref_atom_value(info, pos, &atomv, error_buf) ||
> + if (pos < 0 || (ret = get_ref_atom_value(info, pos, &atomv, error_buf)) ||
> atomv->handler(atomv, &state, error_buf)) {
> pop_stack_element(&state.stack);
... and use that mental energy on readist stuff like this, which I'd
just leave as the inline assignment.
