builtin/repack.c: be more conservative with unsigned overflows

There are a number of places in the geometric repack code where we
multiply the number of objects in a pack by another unsigned value. We
trust that the number of objects in a pack is always representable by a
uint32_t, but we don't necessarily trust that that number can be
multiplied without overflow.

Sprinkle some unsigned_add_overflows() and unsigned_mult_overflows() in
split_pack_geometry() to check that we never overflow any unsigned types
when adding or multiplying them.

Arguably these checks are a little too conservative, and certainly they
do not help the readability of this function. But they are serving a
useful purpose, so I think they are worthwhile overall.

Suggested-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/builtin/repack.c b/builtin/repack.c
index 21a5778e73e31506aaf36b7c4fe44a285de87cf9..677c6b75c1ce219ae2300eb3ffb2b6d3b91b3ccc 100644 (file)
--- a/builtin/repack.c
+++ b/builtin/repack.c
@@ -363,6 +363,12 @@ static void split_pack_geometry(struct pack_geometry *geometry, int factor)
        for (i = geometry->pack_nr - 1; i > 0; i--) {
                struct packed_git *ours = geometry->pack[i];
                struct packed_git *prev = geometry->pack[i - 1];
+
+               if (unsigned_mult_overflows(factor, geometry_pack_weight(prev)))
+                       die(_("pack %s too large to consider in geometric "
+                             "progression"),
+                           prev->pack_name);
+
                if (geometry_pack_weight(ours) < factor * geometry_pack_weight(prev))
                        break;
        }
@@ -388,11 +394,25 @@ static void split_pack_geometry(struct pack_geometry *geometry, int factor)
         * packs in the heavy half need to be joined into it (if any) to restore
         * the geometric progression.
         */
-       for (i = 0; i < split; i++)
-               total_size += geometry_pack_weight(geometry->pack[i]);
+       for (i = 0; i < split; i++) {
+               struct packed_git *p = geometry->pack[i];
+
+               if (unsigned_add_overflows(total_size, geometry_pack_weight(p)))
+                       die(_("pack %s too large to roll up"), p->pack_name);
+               total_size += geometry_pack_weight(p);
+       }
        for (i = split; i < geometry->pack_nr; i++) {
                struct packed_git *ours = geometry->pack[i];
+
+               if (unsigned_mult_overflows(factor, total_size))
+                       die(_("pack %s too large to roll up"), ours->pack_name);
+
                if (geometry_pack_weight(ours) < factor * total_size) {
+                       if (unsigned_add_overflows(total_size,
+                                                  geometry_pack_weight(ours)))
+                               die(_("pack %s too large to roll up"),
+                                   ours->pack_name);
+
                        split++;
                        total_size += geometry_pack_weight(ours);
                } else