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...
> @@ -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:
> - 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
