Factor out realloc handling to a separate helper function

nih-at · Dec 20, 2022 · 079de65 · 079de65
1 parent dbaac5c
commit 079de65
Show file tree

Hide file tree

Showing 7 changed files with 103 additions and 51 deletions.
diff --git a/lib/CMakeLists.txt b/lib/CMakeLists.txt
@@ -60,6 +60,7 @@ add_library(zip
   zip_open.c
   zip_pkware.c
   zip_progress.c
+  zip_realloc.c
   zip_rename.c
   zip_replace.c
   zip_set_archive_comment.c

diff --git a/lib/zip_add_entry.c b/lib/zip_add_entry.c
@@ -44,32 +44,21 @@ _zip_add_entry(zip_t *za) {
     zip_uint64_t idx;
 
     if (za->nentry + 1 >= za->nentry_alloc) {
-        zip_entry_t *rentries;
-        zip_uint64_t nalloc = za->nentry_alloc;
-        zip_uint64_t additional_entries = 2 * nalloc;
-        zip_uint64_t realloc_size;
+        zip_uint64_t additional_entries = 2 * za->nentry_alloc;
+        int result;
 
         if (additional_entries < 16) {
             additional_entries = 16;
         }
         else if (additional_entries > 1024) {
             additional_entries = 1024;
         }
-        /* neither + nor * overflows can happen: nentry_alloc * sizeof(struct zip_entry) < UINT64_MAX */
-        nalloc += additional_entries;
-        realloc_size = sizeof(struct zip_entry) * (size_t)nalloc;
 
-        if (sizeof(struct zip_entry) * (size_t)za->nentry_alloc > realloc_size) {
-            zip_error_set(&za->error, ZIP_ER_MEMORY, 0);
+        result = _zip_realloc(&za->entry, &za->nentry_alloc, sizeof(struct zip_entry), additional_entries);
+        if (result != ZIP_ER_OK) {
+            zip_error_set(&za->error, result, 0);
             return -1;
         }
-        rentries = (zip_entry_t *)realloc(za->entry, sizeof(struct zip_entry) * (size_t)nalloc);
-        if (!rentries) {
-            zip_error_set(&za->error, ZIP_ER_MEMORY, 0);
-            return -1;
-        }
-        za->entry = rentries;
-        za->nentry_alloc = nalloc;
     }
 
     idx = za->nentry++;

diff --git a/lib/zip_dirent.c b/lib/zip_dirent.c
@@ -88,30 +88,23 @@ bool
 _zip_cdir_grow(zip_cdir_t *cd, zip_uint64_t additional_entries, zip_error_t *error) {
     zip_uint64_t i, new_alloc;
     zip_entry_t *new_entry;
+    int result;
 
     if (additional_entries == 0) {
         return true;
     }
 
-    new_alloc = cd->nentry_alloc + additional_entries;
-
-    if (new_alloc < additional_entries || new_alloc > SIZE_MAX / sizeof(*(cd->entry))) {
-        zip_error_set(error, ZIP_ER_MEMORY, 0);
+    result = _zip_realloc(&cd->entry, &cd->nentry_alloc, sizeof(*(cd->entry)), additional_entries);
+    if (result != ZIP_ER_OK) {
+        zip_error_set(error, result, 0);
         return false;
     }
 
-    if ((new_entry = (zip_entry_t *)realloc(cd->entry, sizeof(*(cd->entry)) * (size_t)new_alloc)) == NULL) {
-        zip_error_set(error, ZIP_ER_MEMORY, 0);
-        return false;
-    }
-
-    cd->entry = new_entry;
-
-    for (i = cd->nentry; i < new_alloc; i++) {
+    for (i = cd->nentry; i < cd->nentry_alloc; i++) {
         _zip_entry_init(cd->entry + i);
     }
 
-    cd->nentry = cd->nentry_alloc = new_alloc;
+    cd->nentry = cd->nentry_alloc;
 
     return true;
 }

diff --git a/lib/zip_realloc.c b/lib/zip_realloc.c
@@ -0,0 +1,66 @@
+/*
+  zip_realloc.c -- reallocate with additional elements
+  Copyright (C) 2009-2022 Dieter Baron and Thomas Klausner
+
+  This file is part of libzip, a library to manipulate ZIP archives.
+  The authors can be contacted at <libzip@nih.at>
+
+  Redistribution and use in source and binary forms, with or without
+  modification, are permitted provided that the following conditions
+  are met:
+  1. Redistributions of source code must retain the above copyright
+     notice, this list of conditions and the following disclaimer.
+  2. Redistributions in binary form must reproduce the above copyright
+     notice, this list of conditions and the following disclaimer in
+     the documentation and/or other materials provided with the
+     distribution.
+  3. The names of the authors may not be used to endorse or promote
+     products derived from this software without specific prior
+     written permission.
+
+  THIS SOFTWARE IS PROVIDED BY THE AUTHORS ``AS IS'' AND ANY EXPRESS
+  OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
+  WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+  ARE DISCLAIMED.  IN NO EVENT SHALL THE AUTHORS BE LIABLE FOR ANY
+  DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
+  DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE
+  GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
+  INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER
+  IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR
+  OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN
+  IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+*/
+
+
+#include <stdlib.h>
+
+#include "zipint.h"
+
+int
+(_zip_realloc)(void **inout_memory, zip_uint64_t *inout_alloced, zip_uint64_t element_size, zip_uint64_t additional_elements) {
+    void *old_memory = *inout_memory;
+    zip_uint64_t old_alloced = *inout_alloced;
+    zip_uint64_t new_alloced;
+    size_t new_size;
+    void *new_memory;
+
+    if (additional_elements == 0)
+        additional_elements = 1;
+
+    if (old_alloced > ZIP_UINT64_MAX - additional_elements)
+        return ZIP_ER_MEMORY;
+
+    new_alloced = old_alloced + additional_elements;
+
+    if (new_alloced > ((zip_uint64_t)SIZE_MAX) / element_size)
+        return ZIP_ER_MEMORY;
+
+    new_size = new_alloced * element_size;
+
+    if ((new_memory = realloc(old_memory, new_size)) == NULL)
+        return ZIP_ER_MEMORY;
+
+    *inout_memory = new_memory;
+    *inout_alloced = new_alloced;
+    return ZIP_ER_OK;
+}
diff --git a/lib/zip_source_buffer.c b/lib/zip_source_buffer.c
@@ -429,30 +429,32 @@ static bool
 buffer_grow_fragments(buffer_t *buffer, zip_uint64_t capacity, zip_error_t *error) {
     zip_buffer_fragment_t *fragments;
     zip_uint64_t *offsets;
+    int result;
+    zip_uint64_t tmp_fragments_capacity, tmp_offsets_capacity, additional_elements;
 
-    if (capacity < buffer->fragments_capacity) {
+    if (capacity <= buffer->fragments_capacity) {
         return true;
     }
 
-    zip_uint64_t fragments_size = sizeof(buffer->fragments[0]) * capacity;
-    zip_uint64_t offsets_size = sizeof(buffer->fragment_offsets[0]) * (capacity + 1);
-
-    if (capacity == ZIP_UINT64_MAX || fragments_size < capacity || fragments_size > SIZE_MAX|| offsets_size < capacity || offsets_size > SIZE_MAX) {
-        zip_error_set(error, ZIP_ER_MEMORY, 0);
+    tmp_fragments_capacity = buffer->fragments_capacity;
+    additional_elements = capacity - buffer->fragments_capacity;
+    result = _zip_realloc(&buffer->fragments, &tmp_fragments_capacity, sizeof(buffer->fragments[0]), additional_elements);
+    if (result != ZIP_ER_OK) {
+        zip_error_set(error, result, 0);
         return false;
     }
 
-    if ((fragments = realloc(buffer->fragments, (size_t)fragments_size)) == NULL) {
-        zip_error_set(error, ZIP_ER_MEMORY, 0);
+    /* technically it's a lie when we have no offsets at all in the
+       beginning, but it is of no consequence - the old capacity is
+       used only for computing the new capacity. */
+    tmp_offsets_capacity = buffer->fragments_capacity + 1;
+    result = _zip_realloc(&buffer->fragment_offsets, &tmp_offsets_capacity, sizeof(buffer->fragment_offsets[0]), additional_elements);
+    if (result != ZIP_ER_OK) {
+        zip_error_set(error, result, 0);
         return false;
     }
-    buffer->fragments = fragments;
-    if ((offsets = realloc(buffer->fragment_offsets, (size_t)offsets_size)) == NULL) {
-        zip_error_set(error, ZIP_ER_MEMORY, 0);
-        return false;
-    }
-    buffer->fragment_offsets = offsets;
-    buffer->fragments_capacity = capacity;
+
+    buffer->fragments_capacity = tmp_fragments_capacity;
 
     return true;
 }

diff --git a/lib/zip_source_window.c b/lib/zip_source_window.c
@@ -320,15 +320,13 @@ _zip_register_source(zip_t *za, zip_source_t *src) {
     zip_source_t **open_source;
 
     if (za->nopen_source + 1 >= za->nopen_source_alloc) {
-        unsigned int n;
-        n = za->nopen_source_alloc + 10;
-        open_source = (zip_source_t **)realloc(za->open_source, n * sizeof(zip_source_t *));
-        if (open_source == NULL) {
-            zip_error_set(&za->error, ZIP_ER_MEMORY, 0);
+        int result;
+
+        result = _zip_realloc(&za->open_source, &za->nopen_source_alloc, sizeof(zip_source_t *), 10);
+        if (result != ZIP_ER_OK) {
+            zip_error_set(&za->error, result, 0);
             return -1;
         }
-        za->nopen_source_alloc = n;
-        za->open_source = open_source;
     }
 
     za->open_source[za->nopen_source++] = src;

diff --git a/lib/zipint.h b/lib/zipint.h
@@ -487,9 +487,12 @@ typedef struct _zip_pkware_keys zip_pkware_keys_t;
 #endif
 #endif
 
+#define _zip_realloc(inout_memory, inout_alloced, element_size, additional_elements) (_zip_realloc)((void **)inout_memory, inout_alloced, element_size, additional_elements)
+int (_zip_realloc)(void **inout_memory, zip_uint64_t *inout_alloced, zip_uint64_t element_size, zip_uint64_t additional_elements);
 
 zip_int64_t _zip_add_entry(zip_t *);
 
+
 zip_uint8_t *_zip_buffer_data(zip_buffer_t *buffer);
 bool _zip_buffer_eof(zip_buffer_t *buffer);
 void _zip_buffer_free(zip_buffer_t *buffer);