Re: [PATCH 6/6] tag.c: use type_from_string_gently() when parsing tags
From: Jeff King <hidden>
Date: 2021-04-09 18:42:24
On Fri, Apr 09, 2021 at 10:32:54AM +0200, Ævar Arnfjörð Bjarmason wrote:
Change a series of strcmp() to instead use type_from_string_gently() to get the integer type early, and then use that for comparison.
The result looks much nicer. One interesting note...
quoted hunk ↗ jump to hunk
@@ -162,23 +162,24 @@ int parse_tag_buffer(struct repository *r, struct tag *item, const void *data, u return -1; bufptr += 5; nl = memchr(bufptr, '\n', tail - bufptr); - if (!nl || sizeof(type) <= (nl - bufptr)) + if (!nl) + return -1; + type = type_from_string_gently(bufptr, nl - bufptr); + if (type < 0) return -1;
If we got anything but the main-4 types here, we'll return an error. So we know here:
quoted hunk ↗ jump to hunk
- if (!strcmp(type, blob_type)) { + if (type == OBJ_BLOB) { item->tagged = (struct object *)lookup_blob(r, &oid); - } else if (!strcmp(type, tree_type)) { + } else if (type == OBJ_TREE) { item->tagged = (struct object *)lookup_tree(r, &oid); - } else if (!strcmp(type, commit_type)) { + } else if (type == OBJ_COMMIT) { item->tagged = (struct object *)lookup_commit(r, &oid); - } else if (!strcmp(type, tag_type)) { + } else if (type == OBJ_TAG) { item->tagged = (struct object *)lookup_tag(r, &oid); } else { return error("unknown tag type '%s' in %s", - type, oid_to_hex(&item->object.oid)); + type_name(type), oid_to_hex(&item->object.oid)); }
That the final "else" clause can't be reached. I don't mind being
defensive, but if it _is_ reached, then we'd feed that unknown type to
type_name(), which will return NULL for unknown items (unless I guess it
has also learned about the hypothetical new type).
I think this should just be:
else {
BUG("type_from_string gave us an unknown type: %d", (int)type);
}
which makes it clear we expect the code can't be reached, and doesn't
make any assumptions about what we can do with the odd value.
-Peff