| Message ID | patch-6.6-3279d67d2b-20210409T082935Z-avarab@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | {tag,object}*.c: refactorings + prep for a larger change | expand |
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
diff --git a/tag.c b/tag.c index 3e18a41841..871c4c9a14 100644 --- a/tag.c +++ b/tag.c @@ -135,7 +135,7 @@ void release_tag_memory(struct tag *t) int parse_tag_buffer(struct repository *r, struct tag *item, const void *data, unsigned long size) { struct object_id oid; - char type[20]; + enum object_type type; const char *bufptr = data; const char *tail = bufptr + size; const char *nl; @@ -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; - memcpy(type, bufptr, nl - bufptr); - type[nl - bufptr] = '\0'; bufptr = nl + 1; - 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)); } if (!item->tagged)
Change a series of strcmp() to instead use type_from_string_gently() to get the integer type early, and then use that for comparison. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> --- tag.c | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-)