[PATCH v10 09/17] object-file.c: don't set "typep" when returning non-zero
- From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
- Date
- Oct 1, 2021, 09:16 UTC
- Message-ID
- <patch-v10-09.17-85a91f43634-20211001T091051Z-avarab@gmail.com>
- In-Reply-To
- <cover-v10-00.17-00000000000-20211001T091051Z-avarab@gmail.com>
When the loose_object_info() function returns an error stop faking up the "oi->typep" to OBJ_BAD. Let the return value of the function itself suffice. This code cleanup simplifies subsequent changes.
That we set this at all is a relic from the past. Before 052fe5eaca9 (sha1_loose_object_info: make type lookup optional, 2013-07-12) we would always return the type_from_string(type) via the parse_sha1_header() function, or -1 (i.e. OBJ_BAD) if we couldn't parse it.
Then in a combination of 46f034483eb (sha1_file: support reading from a loose object of unknown type, 2015-05-03) and b3ea7dd32d6 (sha1_loose_object_info: handle errors from unpack_sha1_rest, 2017-10-05) our API drifted even further towards conflating the two again.
Having read the code paths involved carefully I think this is OK. We are just about to return -1, and we have only one caller: do_oid_object_info_extended(). That function will in turn go on to return -1 when we return -1 here.
This might be introducing a subtle bug where a caller of oid_object_info_extended() would inspect its "typep" and expect a meaningful value if the function returned -1.
Such a problem would not occur for its simpler oid_object_info() sister function. That one always returns the "enum object_type", which in the case of -1 would be the OBJ_BAD.
Having read the code for all the callers of these functions I don't believe any such bug is being introduced here, and in any case we'd likely already have such a bug for the "sizep" member (although blindly checking "typep" first would be a more common case).
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> --- object-file.c | 2 -- 1 file changed, 2 deletions(-)
diff --git a/object-file.c b/object-file.c index be4f94ecf3b..766ba88b851 100644 --- a/object-file.c +++ b/object-file.c @@ -1525,8 +1525,6 @@ static int loose_object_info(struct repository *r, git_inflate_end(&stream); munmap(map, mapsize); - if (status && oi->typep) - *oi->typep = status; if (oi->sizep == &size_scratch) oi->sizep = NULL; strbuf_release(&hdrbuf);
-- 2.33.0.1375.g5eed55aa1b5