@matheusbsbs / matheus-bagliano / commits / 123e8186a7

object-file: always set OI_LOOSE when reading object info

There are some early returns in `odb_source_loose_read_object_info()` in cases where we don't have to open the loose object. These return paths do not set `struct object_info::whence` to `OI_LOOSE` though, so it becomes impossible for the caller to tell the format of such an object. The root cause of this really is that we have so many different return paths in the function. As a consequence, it's harder than necessary to make sure that all successful exit paths sot up the `whence` field as expected. Address this by refactoring the function to have a single exit path. Like this, we can trivially set up the `whence` field when we exit successfully from the function. Note that we also: - Rename `status` to `ret` to match our usual coding style, but also to show that the old `status` variable is now always getting the expected value. Furthermore, the value is not initialized anymore, which has the consequence that most compilers will warn for exit paths where we forgot to set it. - Move the setup of scratch pointers closer to `parse_loose_header()` to show where it's needed. - Guard a couple of variables on cleanup so that they only get released in case they have been set up. - Reset `oi->delta_base_oid` towards the end of the function, together with all the other object info pointers. Overall, all these changes result in a diff that is somewhat hard to read. But the end result is significantly easier to read and reason about, so I'd argue this one-time churn is worth it. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Patrick Steinhardt committed Jan 12, 2026 at 10:00 UTC 123e8186a72cd71863668b6acb23a2faa30e6968
1 file changed +71 -44
object-file.c
+71 -44
@@ -416,19 +416,16 @@ int odb_source_loose_read_object_info(struct odb_source *source,
416 const struct object_id *oid,
417 struct object_info *oi, int flags)
418 {
419 - int status = 0;
419 + int ret;
420 int fd;
421 unsigned long mapsize;
422 const char *path;
423 - void *map;
424 - git_zstream stream;
423 + void *map = NULL;
424 + git_zstream stream, *stream_to_end = NULL;
425 char hdr[MAX_HEADER_LEN];
426 unsigned long size_scratch;
427 enum object_type type_scratch;
428
429 - if (oi && oi->delta_base_oid)
430 - oidclr(oi->delta_base_oid, source->odb->repo->hash_algo);
431 -
429 /*
430 * If we don't care about type or size, then we don't
431 * need to look inside the object at all. Note that we
@@ -439,71 +436,101 @@ int odb_source_loose_read_object_info(struct odb_source *source,
436 */
437 if (!oi || (!oi->typep && !oi->sizep && !oi->contentp)) {
438 struct stat st;
442 - if ((!oi || !oi->disk_sizep) && (flags & OBJECT_INFO_QUICK))
443 - return quick_has_loose(source->loose, oid) ? 0 : -1;
444 - if (stat_loose_object(source->loose, oid, &st, &path) < 0)
445 - return -1;
439 +
440 + if ((!oi || !oi->disk_sizep) && (flags & OBJECT_INFO_QUICK)) {
441 + ret = quick_has_loose(source->loose, oid) ? 0 : -1;
442 + goto out;
443 + }
444 +
445 + if (stat_loose_object(source->loose, oid, &st, &path) < 0) {
446 + ret = -1;
447 + goto out;
448 + }
449 +
450 if (oi && oi->disk_sizep)
451 *oi->disk_sizep = st.st_size;
448 - return 0;
452 +
453 + ret = 0;
454 + goto out;
455 }
456
457 fd = open_loose_object(source->loose, oid, &path);
458 if (fd < 0) {
459 if (errno != ENOENT)
460 error_errno(_("unable to open loose object %s"), oid_to_hex(oid));
455 - return -1;
461 + ret = -1;
462 + goto out;
463 }
457 - map = map_fd(fd, path, &mapsize);
458 - if (!map)
459 - return -1;
464
461 - if (!oi->sizep)
462 - oi->sizep = &size_scratch;
463 - if (!oi->typep)
464 - oi->typep = &type_scratch;
465 + map = map_fd(fd, path, &mapsize);
466 + if (!map) {
467 + ret = -1;
468 + goto out;
469 + }
470
471 if (oi->disk_sizep)
472 *oi->disk_sizep = mapsize;
473
474 + stream_to_end = &stream;
475 +
476 switch (unpack_loose_header(&stream, map, mapsize, hdr, sizeof(hdr))) {
477 case ULHR_OK:
471 - if (parse_loose_header(hdr, oi) < 0)
472 - status = error(_("unable to parse %s header"), oid_to_hex(oid));
473 - else if (*oi->typep < 0)
478 + if (!oi->sizep)
479 + oi->sizep = &size_scratch;
480 + if (!oi->typep)
481 + oi->typep = &type_scratch;
482 +
483 + if (parse_loose_header(hdr, oi) < 0) {
484 + ret = error(_("unable to parse %s header"), oid_to_hex(oid));
485 + goto corrupt;
486 + }
487 +
488 + if (*oi->typep < 0)
489 die(_("invalid object type"));
490
476 - if (!oi->contentp)
477 - break;
478 - *oi->contentp = unpack_loose_rest(&stream, hdr, *oi->sizep, oid);
479 - if (*oi->contentp)
480 - goto cleanup;
491 + if (oi->contentp) {
492 + *oi->contentp = unpack_loose_rest(&stream, hdr, *oi->sizep, oid);
493 + if (!*oi->contentp) {
494 + ret = -1;
495 + goto corrupt;
496 + }
497 + }
498
482 - status = -1;
499 break;
500 case ULHR_BAD:
485 - status = error(_("unable to unpack %s header"),
486 - oid_to_hex(oid));
487 - break;
501 + ret = error(_("unable to unpack %s header"),
502 + oid_to_hex(oid));
503 + goto corrupt;
504 case ULHR_TOO_LONG:
489 - status = error(_("header for %s too long, exceeds %d bytes"),
490 - oid_to_hex(oid), MAX_HEADER_LEN);
491 - break;
505 + ret = error(_("header for %s too long, exceeds %d bytes"),
506 + oid_to_hex(oid), MAX_HEADER_LEN);
507 + goto corrupt;
508 }
509
494 - if (status && (flags & OBJECT_INFO_DIE_IF_CORRUPT))
510 + ret = 0;
511 +
512 +corrupt:
513 + if (ret && (flags & OBJECT_INFO_DIE_IF_CORRUPT))
514 die(_("loose object %s (stored in %s) is corrupt"),
515 oid_to_hex(oid), path);
516
498 -cleanup:
499 - git_inflate_end(&stream);
500 - munmap(map, mapsize);
501 - if (oi->sizep == &size_scratch)
502 - oi->sizep = NULL;
503 - if (oi->typep == &type_scratch)
504 - oi->typep = NULL;
505 - oi->whence = OI_LOOSE;
506 - return status;
517 +out:
518 + if (stream_to_end)
519 + git_inflate_end(stream_to_end);
520 + if (map)
521 + munmap(map, mapsize);
522 + if (oi) {
523 + if (oi->sizep == &size_scratch)
524 + oi->sizep = NULL;
525 + if (oi->typep == &type_scratch)
526 + oi->typep = NULL;
527 + if (oi->delta_base_oid)
528 + oidclr(oi->delta_base_oid, source->odb->repo->hash_algo);
529 + if (!ret)
530 + oi->whence = OI_LOOSE;
531 + }
532 +
533 + return ret;
534 }
535
536 static void hash_object_body(const struct git_hash_algo *algo, struct git_hash_ctx *c,