Finish removing any printing from ref-filter formatting logic, so that it could be more general. Change the signature of get_ref_atom_value() and underlying functions by adding return value and strbuf parameter for error message. It's important to mention that grab_objectname() returned 1 if it gets objectname atom and 0 otherwise. Now this logic changed: we return 0 if we have no error, -1 otherwise. If someone needs to know whether it's objectname atom or not, he/she could use starts_with() function. It duplicates this checking but it does not sound like a really big overhead. Signed-off-by: Olga Telezhnaia <olyatelezhnaya@xxxxxxxxx> --- ref-filter.c | 109 +++++++++++++++++++++++++++++++++++++---------------------- 1 file changed, 69 insertions(+), 40 deletions(-) diff --git a/ref-filter.c b/ref-filter.c index 62ea4adcd0ff1..3f0c3924273d5 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -831,26 +831,27 @@ static void *get_obj(const struct object_id *oid, struct object **obj, unsigned } static int grab_objectname(const char *name, const unsigned char *sha1, - struct atom_value *v, struct used_atom *atom) + struct atom_value *v, struct used_atom *atom, + struct strbuf *err) { if (starts_with(name, "objectname")) { if (atom->u.objectname.option == O_SHORT) { v->s = xstrdup(find_unique_abbrev(sha1, DEFAULT_ABBREV)); - return 1; } else if (atom->u.objectname.option == O_FULL) { v->s = xstrdup(sha1_to_hex(sha1)); - return 1; } else if (atom->u.objectname.option == O_LENGTH) { v->s = xstrdup(find_unique_abbrev(sha1, atom->u.objectname.length)); - return 1; - } else - die("BUG: unknown %%(objectname) option"); + } else { + strbuf_addstr(err, "BUG: unknown %(objectname) option"); + return -1; + } } return 0; } /* See grab_values */ -static void grab_common_values(struct atom_value *val, int deref, struct object *obj, void *buf, unsigned long sz) +static int grab_common_values(struct atom_value *val, int deref, struct object *obj, + void *buf, unsigned long sz, struct strbuf *err) { int i; @@ -868,8 +869,10 @@ static void grab_common_values(struct atom_value *val, int deref, struct object v->s = xstrfmt("%lu", sz); } else if (deref) - grab_objectname(name, obj->oid.hash, v, &used_atom[i]); + if (grab_objectname(name, obj->oid.hash, v, &used_atom[i], err)) + return -1; } + return 0; } /* See grab_values */ @@ -1225,9 +1228,11 @@ static void fill_missing_values(struct atom_value *val) * pointed at by the ref itself; otherwise it is the object the * ref (which is a tag) refers to. */ -static void grab_values(struct atom_value *val, int deref, struct object *obj, void *buf, unsigned long sz) +static int grab_values(struct atom_value *val, int deref, struct object *obj, + void *buf, unsigned long sz, struct strbuf *err) { - grab_common_values(val, deref, obj, buf, sz); + if (grab_common_values(val, deref, obj, buf, sz, err)) + return -1; switch (obj->type) { case OBJ_TAG: grab_tag_values(val, deref, obj, buf, sz); @@ -1247,8 +1252,10 @@ static void grab_values(struct atom_value *val, int deref, struct object *obj, v /* grab_blob_values(val, deref, obj, buf, sz); */ break; default: - die("Eh? Object of type %d?", obj->type); + strbuf_addf(err, "Eh? Object of type %d?", obj->type); + return -1; } + return 0; } static inline char *copy_advance(char *dst, const char *src) @@ -1335,8 +1342,9 @@ static const char *show_ref(struct refname_atom *atom, const char *refname) return refname; } -static void fill_remote_ref_details(struct used_atom *atom, const char *refname, - struct branch *branch, const char **s) +static int fill_remote_ref_details(struct used_atom *atom, const char *refname, + struct branch *branch, const char **s, + struct strbuf *err) { int num_ours, num_theirs; if (atom->u.remote_ref.option == RR_REF) @@ -1362,7 +1370,7 @@ static void fill_remote_ref_details(struct used_atom *atom, const char *refname, } else if (atom->u.remote_ref.option == RR_TRACKSHORT) { if (stat_tracking_info(branch, &num_ours, &num_theirs, NULL, AHEAD_BEHIND_FULL) < 0) - return; + return 0; if (!num_ours && !num_theirs) *s = "="; @@ -1391,8 +1399,11 @@ static void fill_remote_ref_details(struct used_atom *atom, const char *refname, *s = xstrdup(merge); else *s = ""; - } else - die("BUG: unhandled RR_* enum"); + } else { + strbuf_addstr(err, "BUG: unhandled RR_* enum"); + return -1; + } + return 0; } char *get_head_description(void) @@ -1447,28 +1458,33 @@ static const char *get_refname(struct used_atom *atom, struct ref_array_item *re return show_ref(&atom->u.refname, ref->refname); } -static void get_object(struct ref_array_item *ref, const struct object_id *oid, - int deref, struct object **obj) +static int get_object(struct ref_array_item *ref, const struct object_id *oid, + int deref, struct object **obj, struct strbuf *err) { int eaten; unsigned long size; void *buf = get_obj(oid, obj, &size, &eaten); - if (!buf) - die(_("missing object %s for %s"), - oid_to_hex(oid), ref->refname); - if (!*obj) - die(_("parse_object_buffer failed on %s for %s"), - oid_to_hex(oid), ref->refname); - - grab_values(ref->value, deref, *obj, buf, size); + if (!buf) { + strbuf_addf(err, _("missing object %s for %s"), oid_to_hex(oid), + ref->refname); + return -1; + } + if (!*obj) { + strbuf_addf(err, _("parse_object_buffer failed on %s for %s"), + oid_to_hex(oid), ref->refname); + return -1; + } + if (grab_values(ref->value, deref, *obj, buf, size, err)) + return -1; if (!eaten) free(buf); + return 0; } /* * Parse the object referred by ref, and grab needed value. */ -static void populate_value(struct ref_array_item *ref) +static int populate_value(struct ref_array_item *ref, struct strbuf *err) { struct object *obj; int i; @@ -1513,8 +1529,9 @@ static void populate_value(struct ref_array_item *ref) branch = branch_get(branch_name); refname = branch_get_upstream(branch, NULL); - if (refname) - fill_remote_ref_details(atom, refname, branch, &v->s); + if (refname && + fill_remote_ref_details(atom, refname, branch, &v->s, err)) + return -1; continue; } else if (atom->u.remote_ref.push) { const char *branch_name; @@ -1530,7 +1547,8 @@ static void populate_value(struct ref_array_item *ref) if (!refname) continue; } - fill_remote_ref_details(atom, refname, branch, &v->s); + if (fill_remote_ref_details(atom, refname, branch, &v->s, err)) + return -1; continue; } else if (starts_with(name, "color:")) { v->s = atom->u.color; @@ -1548,7 +1566,9 @@ static void populate_value(struct ref_array_item *ref) v->s = xstrdup(buf + 1); } continue; - } else if (!deref && grab_objectname(name, ref->objectname.hash, v, atom)) { + } else if (!deref && starts_with(name, "objectname")) { + if (grab_objectname(name, ref->objectname.hash, v, atom, err)) + return -1; continue; } else if (!strcmp(name, "HEAD")) { if (atom->u.head && !strcmp(ref->refname, atom->u.head)) @@ -1590,16 +1610,17 @@ static void populate_value(struct ref_array_item *ref) break; } if (used_atom_cnt <= i) - return; + return 0; - get_object(ref, &ref->objectname, 0, &obj); + if (get_object(ref, &ref->objectname, 0, &obj, err)) + return -1; /* * If there is no atom that wants to know about tagged * object, we are done. */ if (!need_tagged || (obj->type != OBJ_TAG)) - return; + return 0; /* * If it is a tag object, see if we use a value that derefs @@ -1613,20 +1634,23 @@ static void populate_value(struct ref_array_item *ref) * is not consistent with what deref_tag() does * which peels the onion to the core. */ - get_object(ref, tagged, 1, &obj); + return get_object(ref, tagged, 1, &obj, err); } /* * Given a ref, return the value for the atom. This lazily gets value * out of the object by calling populate value. */ -static void get_ref_atom_value(struct ref_array_item *ref, int atom, struct atom_value **v) +static int get_ref_atom_value(struct ref_array_item *ref, int atom, + struct atom_value **v, struct strbuf *err) { if (!ref->value) { - populate_value(ref); + if (populate_value(ref, err)) + return -1; fill_missing_values(ref->value); } *v = &ref->value[atom]; + return 0; } /* @@ -2150,9 +2174,13 @@ static int cmp_ref_sorting(struct ref_sorting *s, struct ref_array_item *a, stru int cmp; cmp_type cmp_type = used_atom[s->atom].type; int (*cmp_fn)(const char *, const char *); + struct strbuf err = STRBUF_INIT; - get_ref_atom_value(a, s->atom, &va); - get_ref_atom_value(b, s->atom, &vb); + if (get_ref_atom_value(a, s->atom, &va, &err)) + die("%s", err.buf); + if (get_ref_atom_value(b, s->atom, &vb, &err)) + die("%s", err.buf); + strbuf_release(&err); cmp_fn = s->ignore_case ? strcasecmp : strcmp; if (s->version) cmp = versioncmp(va->s, vb->s); @@ -2231,7 +2259,8 @@ int format_ref_array_item(struct ref_array_item *info, append_literal(cp, sp, &state); if (parse_ref_filter_atom(format, sp + 2, ep, &pos, error_buf)) return -1; - get_ref_atom_value(info, pos, &atomv); + if (get_ref_atom_value(info, pos, &atomv, error_buf)) + return -1; if (atomv->handler(atomv, &state, error_buf)) return -1; } -- https://github.com/git/git/pull/466