odb: do not use "blank" substitute for NULL

When various *object_info() functions are given an extended object
info structure as NULL by a caller that does not want any details,
the code uses a file-scope static blank_oi and passes it down to
the helper functions they use, to avoid handling NULL specifically.

The ps/object-read-stream topic graduated to 'master' recently
however had a bug that assumed that two identically named file-scope
static variables in two functions are the same, which of course is
not the case.  This made "git commit" take 0.38 seconds to 1508
seconds in some case, as reported by Aaron Plattner here:

  https://lore.kernel.org/git/f4ba7e89-4717-4b36-921f-56537131fd69@nvidia.com/

We _could_ move the blank_oi variable to the global scope in common
section to fix this regression, but explicitly handling the NULL is
a much safer fix.  It would also reduce the chance of errors that
somebody accidentally writes into blank_oi, making its contents
dirty, which potentially will make subsequent calls into the
function misbehave.  By explicitly handling NULL input, we no longer
have to worry about it.

Reported-by: Aaron Plattner <aplattner@nvidia.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/object-file.c b/object-file.c
index 12177a7dd707a8f4efa46834f7e4e0a3ee8ddc4f..e0cce3a62a827a4a59ad92d52ac1862283970272 100644 (file)
--- a/object-file.c
+++ b/object-file.c
@@ -426,7 +426,7 @@ int odb_source_loose_read_object_info(struct odb_source *source,
        unsigned long size_scratch;
        enum object_type type_scratch;
 
-       if (oi->delta_base_oid)
+       if (oi && oi->delta_base_oid)
                oidclr(oi->delta_base_oid, source->odb->repo->hash_algo);
 
        /*
@@ -437,13 +437,13 @@ int odb_source_loose_read_object_info(struct odb_source *source,
         * return value implicitly indicates whether the
         * object even exists.
         */
-       if (!oi->typep && !oi->sizep && !oi->contentp) {
+       if (!oi || (!oi->typep && !oi->sizep && !oi->contentp)) {
                struct stat st;
-               if (!oi->disk_sizep && (flags & OBJECT_INFO_QUICK))
+               if ((!oi || !oi->disk_sizep) && (flags & OBJECT_INFO_QUICK))
                        return quick_has_loose(source->loose, oid) ? 0 : -1;
                if (stat_loose_object(source->loose, oid, &st, &path) < 0)
                        return -1;
-               if (oi->disk_sizep)
+               if (oi && oi->disk_sizep)
                        *oi->disk_sizep = st.st_size;
                return 0;
        }

diff --git a/odb.c b/odb.c
index f4cbee4b042d83e4442ba3e264b3752e68148bee..85dc21b104d1836a6c7cacd269fa811b337706d2 100644 (file)
--- a/odb.c
+++ b/odb.c
@@ -664,34 +664,31 @@ static int do_oid_object_info_extended(struct object_database *odb,
                                       const struct object_id *oid,
                                       struct object_info *oi, unsigned flags)
 {
-       static struct object_info blank_oi = OBJECT_INFO_INIT;
        const struct cached_object *co;
        const struct object_id *real = oid;
        int already_retried = 0;
 
-
        if (flags & OBJECT_INFO_LOOKUP_REPLACE)
                real = lookup_replace_object(odb->repo, oid);
 
        if (is_null_oid(real))
                return -1;
 
-       if (!oi)
-               oi = &blank_oi;
-
        co = find_cached_object(odb, real);
        if (co) {
-               if (oi->typep)
-                       *(oi->typep) = co->type;
-               if (oi->sizep)
-                       *(oi->sizep) = co->size;
-               if (oi->disk_sizep)
-                       *(oi->disk_sizep) = 0;
-               if (oi->delta_base_oid)
-                       oidclr(oi->delta_base_oid, odb->repo->hash_algo);
-               if (oi->contentp)
-                       *oi->contentp = xmemdupz(co->buf, co->size);
-               oi->whence = OI_CACHED;
+               if (oi) {
+                       if (oi->typep)
+                               *(oi->typep) = co->type;
+                       if (oi->sizep)
+                               *(oi->sizep) = co->size;
+                       if (oi->disk_sizep)
+                               *(oi->disk_sizep) = 0;
+                       if (oi->delta_base_oid)
+                               oidclr(oi->delta_base_oid, odb->repo->hash_algo);
+                       if (oi->contentp)
+                               *oi->contentp = xmemdupz(co->buf, co->size);
+                       oi->whence = OI_CACHED;
+               }
                return 0;
        }
 

diff --git a/packfile.c b/packfile.c
index 7a16aaa90d0a2f817d2de8a5b2aac5b594f7ecfc..2aa6135c3a1fe4d46c72075787068439ba16c58b 100644 (file)
--- a/packfile.c
+++ b/packfile.c
@@ -2095,7 +2095,6 @@ int packfile_store_read_object_info(struct packfile_store *store,
                                    struct object_info *oi,
                                    unsigned flags UNUSED)
 {
-       static struct object_info blank_oi = OBJECT_INFO_INIT;
        struct pack_entry e;
        int rtype;
 
@@ -2106,7 +2105,7 @@ int packfile_store_read_object_info(struct packfile_store *store,
         * We know that the caller doesn't actually need the
         * information below, so return early.
         */
-       if (oi == &blank_oi)
+       if (!oi)
                return 0;
 
        rtype = packed_object_info(store->odb->repo, e.p, e.offset, oi);