From dccb32bf01411213297cde63cf689e476673a8ec Mon Sep 17 00:00:00 2001 From: Ævar Arnfjörð Bjarmason Date: Fri, 1 Oct 2021 11:16:51 +0200 Subject: object-file.c: stop dying in parse_loose_header() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Make parse_loose_header() return error codes and data instead of invoking die() by itself. For now we'll move the relevant die() call to loose_object_info() and read_loose_object() to keep this change smaller. In a subsequent commit we'll make read_loose_object() return an error code instead of dying. We should also address the "allow_unknown" case (should be moved to builtin/cat-file.c), but for now I'll be leaving it. For making parse_loose_header() not die() change its prototype to accept a "struct object_info *" instead of the "unsigned long *sizep" it accepted before. Its callers can now check the populated populated "oi->typep". Because of this we don't need to pass in the "unsigned int flags" which we used for OBJECT_INFO_ALLOW_UNKNOWN_TYPE, we can instead do that check in loose_object_info(). This also refactors some confusing control flow around the "status" variable. In some cases we set it to the return value of "error()", i.e. -1, and later checked if "status < 0" was true. Since 93cff9a978e (sha1_loose_object_info: return error for corrupted objects, 2017-04-01) the return value of loose_object_info() (then named sha1_loose_object_info()) had been a "status" variable that be any negative value, as we were expecting to return the "enum object_type". The only negative type happens to be OBJ_BAD, but the code still assumed that more might be added. This was then used later in e.g. c84a1f3ed4d (sha1_file: refactor read_object, 2017-06-21). Now that parse_loose_header() will return 0 on success instead of the type (which it'll stick into the "struct object_info") we don't need to conflate these two cases in its callers. Since parse_loose_header() doesn't need to return an arbitrary "status" we only need to treat its "ret < 0" specially, but can idiomatically overwrite it with our own error() return. This along with having made unpack_loose_header() return an "enum unpack_loose_header_result" in an earlier commit means that we can move the previously nested if/else cases mostly into the "ULHR_OK" branch of the "switch" statement. We should be less silent if we reach that "status = -1" branch, which happens if we've got trailing garbage in loose objects, see f6371f92104 (sha1_file: add read_loose_object() function, 2017-01-13) for a better way to handle it. For now let's punt on it, a subsequent commit will address that edge case. Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano --- cache.h | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) (limited to 'cache.h') diff --git a/cache.h b/cache.h index e7d0cc3d3b..1181304f3f 100644 --- a/cache.h +++ b/cache.h @@ -1332,9 +1332,16 @@ enum unpack_loose_header_result unpack_loose_header(git_zstream *stream, unsigned long bufsiz, struct strbuf *hdrbuf); +/** + * parse_loose_header() parses the starting " \0" of an + * object. If it doesn't follow that format -1 is returned. To check + * the validity of the populate the "typep" in the "struct + * object_info". It will be OBJ_BAD if the object type is unknown. The + * parsed can be retrieved via "oi->sizep", and from there + * passed to unpack_loose_rest(). + */ struct object_info; -int parse_loose_header(const char *hdr, struct object_info *oi, - unsigned int flags); +int parse_loose_header(const char *hdr, struct object_info *oi); int check_object_signature(struct repository *r, const struct object_id *oid, void *buf, unsigned long size, const char *type); -- cgit v1.3