Jonathan Tan <jonathantanmy@xxxxxxxxxx> writes:
> diff --git a/object-file.c b/object-file.c
> index 429e3a746d..e0cef8b906 100644
> --- a/object-file.c
> +++ b/object-file.c
> @@ -1422,7 +1422,9 @@ static int loose_object_info(struct repository *r,
> struct object_info *oi, int flags)
> {
> int status = 0;
> + int fd;
> unsigned long mapsize;
> + const char *path = NULL;
It may be OK to leave this uninitialized, as long as the contract of
open_loose_object() is that a successful opening will report the path
to the loose object file that was opened. Because ...
> @@ -1454,7 +1455,13 @@ static int loose_object_info(struct repository *r,
> return 0;
> }
>
> - map = map_loose_object(r, oid, &mapsize);
> + fd = open_loose_object(r, oid, &path);
> + if (fd < 0) {
> + if (errno != ENOENT)
> + error_errno(_("unable to open loose object %s"), oid_to_hex(oid));
> + return -1;
... we no longer refer to "path" which may not be populated here, and ...
> + }
... at this point, we know we successfully opened, and populated path.
> + map = map_fd(fd, path, &mapsize);
> if (!map)
> return -1;
>
> @@ -1492,6 +1499,10 @@ static int loose_object_info(struct repository *r,
> break;
> }
>
> + if (status && path && (flags & OBJECT_INFO_DIE_IF_CORRUPT))
Also, here, "path" should be valid, as we have successfully opened
the loose object file (otherwise we would have hit the early return
that reports only the oid_to_hex(oid)).
> + die(_("loose object %s (stored in %s) is corrupt"),
> + oid_to_hex(oid), path);
If we didn't have path and did not die, we'd end up ignoring
DIE_IF_CORRUPT request and force the caller to handle non-zero
status. But I do not think that should happen, because path would
be valid here. No?
