Re: [PATCH 5/8] [GSOC] ref-filter: teach get_object() return useful value
- From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
- Date
- Jun 17, 2021, 07:22 UTC
- Message-ID
- <87wnqtvtph.fsf@evledraar.gmail.com>
- In-Reply-To
- <c208b8a45d66556a3f905063bc7c5026ac4f1e82.1623496458.git.gitgitgadget@gmail.com>
On Sat, Jun 12 2021, ZheNing Hu via GitGitGadget wrote:
Show 24 quoted lines
> From: ZheNing Hu <adlternative@gmail.com>
>
> 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@gmail.com>
> Mentored-by: Hariom Verma <hariom18599@gmail.com>
> Signed-off-by: ZheNing Hu <adlternative@gmail.com>
> ---
> 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)
Show 11 quoted lines
> 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...
Show 8 quoted lines
> @@ -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.