git@vger.kernel.org mailing list mirror (one of many)
 help / Atom feed
From: Olga Telezhnaya <olyatelezhnaya@gmail.com>
To: git@vger.kernel.org
Subject: [PATCH v2 5/5] ref-filter: get_ref_atom_value() error handling
Date: Wed, 14 Mar 2018 19:04:20 +0000
Message-ID: <0102016225e61cc2-0d0c05ef-b999-4bf2-88ac-db9840dd2a87-000000@eu-west-1.amazonses.com> (raw)
In-Reply-To: <0102016225e61c34-50ab78f0-6fb6-4ece-acfe-da253bdce616-000000@eu-west-1.amazonses.com>

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@gmail.com>
---
 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

  parent reply index

Thread overview: 53+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-03-13 10:16 [RFC 1/4] ref-filter: start adding strbufs with errors Olga Telezhnaya
2018-03-13 10:16 ` [RFC 3/4] ref-filter: change parsing function error handling Olga Telezhnaya
2018-03-13 19:18   ` Martin Ågren
2018-03-14 13:36     ` Оля Тележная
2018-03-13 10:16 ` [RFC 2/4] ref-filter: add return value && strbuf to handlers Olga Telezhnaya
2018-03-13 19:13   ` Martin Ågren
2018-03-13 10:16 ` [RFC 4/4] ref-filter: add return value to parsers Olga Telezhnaya
2018-03-13 19:12 ` [RFC 1/4] ref-filter: start adding strbufs with errors Martin Ågren
2018-03-14 13:30   ` Оля Тележная
2018-03-14 19:04 ` [PATCH v2 1/5] " Olga Telezhnaya
2018-03-14 19:04   ` [PATCH v2 4/5] ref-filter: add return value to parsers Olga Telezhnaya
2018-03-14 19:04   ` Olga Telezhnaya [this message]
2018-03-15 20:47     ` [PATCH v2 5/5] ref-filter: get_ref_atom_value() error handling Martin Ågren
2018-03-15 21:01       ` Eric Sunshine
2018-03-16  7:20         ` Оля Тележная
2018-03-15 21:05       ` Junio C Hamano
2018-03-16  7:17       ` Оля Тележная
2018-03-14 19:04   ` [PATCH v2 2/5] ref-filter: add return value && strbuf to handlers Olga Telezhnaya
2018-03-14 19:04   ` [PATCH v2 3/5] ref-filter: change parsing function error handling Olga Telezhnaya
2018-03-15 22:48     ` Junio C Hamano
2018-03-16  7:22       ` Оля Тележная
2018-03-19 13:01   ` [PATCH v3 1/5] ref-filter: start adding strbufs with errors Olga Telezhnaya
2018-03-19 13:01     ` [PATCH v3 4/5] ref-filter: add return value to parsers Olga Telezhnaya
2018-03-19 13:01     ` [PATCH v3 2/5] ref-filter: add return value && strbuf to handlers Olga Telezhnaya
2018-03-19 19:24       ` Junio C Hamano
2018-03-19 13:01     ` [PATCH v3 5/5] ref-filter: get_ref_atom_value() error handling Olga Telezhnaya
2018-03-19 13:01     ` [PATCH v3 3/5] ref-filter: change parsing function " Olga Telezhnaya
2018-03-19 19:41       ` Junio C Hamano
2018-03-20 16:05     ` [PATCH v4 1/5] ref-filter: start adding strbufs with errors Olga Telezhnaya
2018-03-20 16:05       ` [PATCH v4 3/5] ref-filter: change parsing function error handling Olga Telezhnaya
2018-03-20 16:05       ` [PATCH v4 2/5] ref-filter: add return value && strbuf to handlers Olga Telezhnaya
2018-03-20 18:18         ` Eric Sunshine
2018-03-20 16:05       ` [PATCH v4 4/5] ref-filter: add return value to parsers Olga Telezhnaya
2018-03-20 16:05       ` [PATCH v4 5/5] ref-filter: get_ref_atom_value() error handling Olga Telezhnaya
2018-03-20 18:19         ` Eric Sunshine
2018-03-20 22:30           ` Junio C Hamano
2018-03-20 22:50             ` Eric Sunshine
2018-03-21 19:30               ` Junio C Hamano
2018-03-21 19:58                 ` Eric Sunshine
2018-03-21 18:28       ` [PATCH v5 1/6] strbuf: add shortcut to work with error messages Olga Telezhnaya
2018-03-21 18:28         ` [PATCH v5 5/6] ref-filter: add return value to parsers Olga Telezhnaya
2018-03-21 18:28         ` [PATCH v5 3/6] ref-filter: add return value && strbuf to handlers Olga Telezhnaya
2018-03-21 18:28         ` [PATCH v5 6/6] ref-filter: libify get_ref_atom_value() Olga Telezhnaya
2018-03-21 18:28         ` [PATCH v5 4/6] ref-filter: change parsing function error handling Olga Telezhnaya
2018-03-21 20:36           ` Junio C Hamano
2018-03-23  6:56             ` Оля Тележная
2018-03-21 18:28         ` [PATCH v5 2/6] ref-filter: start adding strbufs with errors Olga Telezhnaya
2018-03-21 20:20         ` [PATCH v5 1/6] strbuf: add shortcut to work with error messages Junio C Hamano
2018-03-23  6:48           ` Оля Тележная
2018-03-29 12:49         ` [PATCH v6 1/6] ref-filter: add shortcut to work with strbufs Olga Telezhnaya
2018-03-29 12:49           ` [PATCH v6 6/6] ref-filter: libify get_ref_atom_value() Olga Telezhnaya
2018-03-29 12:49           ` [PATCH v6 5/6] ref-filter: add return value to parsers Olga Telezhnaya
2018-03-29 12:49           ` [PATCH v6 2/6] ref-filter: start adding strbufs with errors Olga Telezhnaya
2018-03-29 12:49           ` [PATCH v6 3/6] ref-filter: add return value && strbuf to handlers Olga Telezhnaya
2018-03-29 12:49           ` [PATCH v6 4/6] ref-filter: change parsing function error handling Olga Telezhnaya

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=0102016225e61cc2-0d0c05ef-b999-4bf2-88ac-db9840dd2a87-000000@eu-west-1.amazonses.com \
    --to=olyatelezhnaya@gmail.com \
    --cc=git@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

git@vger.kernel.org mailing list mirror (one of many)

Archives are clonable:
	git clone --mirror https://public-inbox.org/git
	git clone --mirror http://ou63pmih66umazou.onion/git
	git clone --mirror http://czquwvybam4bgbro.onion/git
	git clone --mirror http://hjrcffqmbrq6wope.onion/git

Newsgroups are available over NNTP:
	nntp://news.public-inbox.org/inbox.comp.version-control.git
	nntp://ou63pmih66umazou.onion/inbox.comp.version-control.git
	nntp://czquwvybam4bgbro.onion/inbox.comp.version-control.git
	nntp://hjrcffqmbrq6wope.onion/inbox.comp.version-control.git
	nntp://news.gmane.org/gmane.comp.version-control.git

 note: .onion URLs require Tor: https://www.torproject.org/
       or Tor2web: https://www.tor2web.org/

AGPL code for this site: git clone https://public-inbox.org/ public-inbox