| Message ID | 20200413052848.1246-1-rikard.falkeborn@gmail.com |
|---|---|
| State | Accepted, archived |
| Headers | show |
| Series | [pacman-dev] Add REALLOC macro to simplify realloc error handling | expand |
On 13/4/20 3:28 pm, Rikard Falkeborn wrote: > realloc can fail just like the other memory allocation functions. Add a > macro to simplify handling of realloc failures, similar to the already > existing MALLOC, CALLOC, etc. > > Replace the existing realloc uses with the new macro, allowing us to > move tedious error handling to the macro. Also, in be_package and > be_sync, this fixes hypothetical memory leaks (and thereafter null > pointer dereferences) in case realloc fails to shrink the allocated > memory. > Ack. > Signed-off-by: Rikard Falkeborn <rikard.falkeborn@gmail.com> > --- > lib/libalpm/be_local.c | 7 +------ > lib/libalpm/be_package.c | 3 +-- > lib/libalpm/be_sync.c | 2 +- > lib/libalpm/util.c | 13 +++---------- > lib/libalpm/util.h | 1 + > 5 files changed, 7 insertions(+), 19 deletions(-) > > diff --git a/lib/libalpm/be_local.c b/lib/libalpm/be_local.c > index 9ebdfa40..689f0102 100644 > --- a/lib/libalpm/be_local.c > +++ b/lib/libalpm/be_local.c > @@ -843,12 +843,7 @@ static int local_db_read(alpm_pkg_t *info, int inforeq) > } > /* attempt to hand back any memory we don't need */ > if(files_count > 0) { > - alpm_file_t *newfiles; > - > - newfiles = realloc(files, sizeof(alpm_file_t) * files_count); > - if(newfiles != NULL) { > - files = newfiles; > - } > + REALLOC(files, sizeof(alpm_file_t) * files_count, (void)0); > } else { > FREE(files); > } > diff --git a/lib/libalpm/be_package.c b/lib/libalpm/be_package.c > index 7a118d2a..4832966b 100644 > --- a/lib/libalpm/be_package.c > +++ b/lib/libalpm/be_package.c > @@ -669,8 +669,7 @@ alpm_pkg_t *_alpm_pkg_load_internal(alpm_handle_t *handle, > if(full) { > if(newpkg->files.files) { > /* attempt to hand back any memory we don't need */ > - newpkg->files.files = realloc(newpkg->files.files, > - sizeof(alpm_file_t) * newpkg->files.count); > + REALLOC(newpkg->files.files, sizeof(alpm_file_t) * newpkg->files.count, (void)0); > /* "checking for conflicts" requires a sorted list, ensure that here */ > _alpm_log(handle, ALPM_LOG_DEBUG, > "sorting package filelist for %s\n", pkgfile); > diff --git a/lib/libalpm/be_sync.c b/lib/libalpm/be_sync.c > index aafed15d..5f457122 100644 > --- a/lib/libalpm/be_sync.c > +++ b/lib/libalpm/be_sync.c > @@ -689,7 +689,7 @@ static int sync_db_read(alpm_db_t *db, struct archive *archive, > } > /* attempt to hand back any memory we don't need */ > if(files_count > 0) { > - files = realloc(files, sizeof(alpm_file_t) * files_count); > + REALLOC(files, sizeof(alpm_file_t) * files_count, (void)0); > } else { > FREE(files); > } > diff --git a/lib/libalpm/util.c b/lib/libalpm/util.c > index c46b1397..cebc87ec 100644 > --- a/lib/libalpm/util.c > +++ b/lib/libalpm/util.c > @@ -1427,22 +1427,15 @@ int _alpm_fnmatch(const void *pattern, const void *string) > */ > void *_alpm_realloc(void **data, size_t *current, const size_t required) > { > - char *newdata; > - > - newdata = realloc(*data, required); > - if(!newdata) { > - _alpm_alloc_fail(required); > - return NULL; > - } > + REALLOC(*data, required, return NULL); > > if (*current < required) { > /* ensure all new memory is zeroed out, in both the initial > * allocation and later reallocs */ > - memset(newdata + *current, 0, required - *current); > + memset((char*)*data + *current, 0, required - *current); > } > *current = required; > - *data = newdata; > - return newdata; > + return *data; > } > > /** This automatically grows data based on current/required. > diff --git a/lib/libalpm/util.h b/lib/libalpm/util.h > index 675eedec..3306a022 100644 > --- a/lib/libalpm/util.h > +++ b/lib/libalpm/util.h > @@ -53,6 +53,7 @@ void _alpm_alloc_fail(size_t size); > > #define MALLOC(p, s, action) do { p = malloc(s); if(p == NULL) { _alpm_alloc_fail(s); action; } } while(0) > #define CALLOC(p, l, s, action) do { p = calloc(l, s); if(p == NULL) { _alpm_alloc_fail(l * s); action; } } while(0) > +#define REALLOC(p, s, action) do { void* np = realloc(p, s); if(np == NULL) { _alpm_alloc_fail(s); action; } else { p = np; } } while(0) > /* This strdup macro is NULL safe- copying NULL will yield NULL */ > #define STRDUP(r, s, action) do { if(s != NULL) { r = strdup(s); if(r == NULL) { _alpm_alloc_fail(strlen(s)); action; } } else { r = NULL; } } while(0) > #define STRNDUP(r, s, l, action) do { if(s != NULL) { r = strndup(s, l); if(r == NULL) { _alpm_alloc_fail(l); action; } } else { r = NULL; } } while(0) >
diff --git a/lib/libalpm/be_local.c b/lib/libalpm/be_local.c index 9ebdfa40..689f0102 100644 --- a/lib/libalpm/be_local.c +++ b/lib/libalpm/be_local.c @@ -843,12 +843,7 @@ static int local_db_read(alpm_pkg_t *info, int inforeq) } /* attempt to hand back any memory we don't need */ if(files_count > 0) { - alpm_file_t *newfiles; - - newfiles = realloc(files, sizeof(alpm_file_t) * files_count); - if(newfiles != NULL) { - files = newfiles; - } + REALLOC(files, sizeof(alpm_file_t) * files_count, (void)0); } else { FREE(files); } diff --git a/lib/libalpm/be_package.c b/lib/libalpm/be_package.c index 7a118d2a..4832966b 100644 --- a/lib/libalpm/be_package.c +++ b/lib/libalpm/be_package.c @@ -669,8 +669,7 @@ alpm_pkg_t *_alpm_pkg_load_internal(alpm_handle_t *handle, if(full) { if(newpkg->files.files) { /* attempt to hand back any memory we don't need */ - newpkg->files.files = realloc(newpkg->files.files, - sizeof(alpm_file_t) * newpkg->files.count); + REALLOC(newpkg->files.files, sizeof(alpm_file_t) * newpkg->files.count, (void)0); /* "checking for conflicts" requires a sorted list, ensure that here */ _alpm_log(handle, ALPM_LOG_DEBUG, "sorting package filelist for %s\n", pkgfile); diff --git a/lib/libalpm/be_sync.c b/lib/libalpm/be_sync.c index aafed15d..5f457122 100644 --- a/lib/libalpm/be_sync.c +++ b/lib/libalpm/be_sync.c @@ -689,7 +689,7 @@ static int sync_db_read(alpm_db_t *db, struct archive *archive, } /* attempt to hand back any memory we don't need */ if(files_count > 0) { - files = realloc(files, sizeof(alpm_file_t) * files_count); + REALLOC(files, sizeof(alpm_file_t) * files_count, (void)0); } else { FREE(files); } diff --git a/lib/libalpm/util.c b/lib/libalpm/util.c index c46b1397..cebc87ec 100644 --- a/lib/libalpm/util.c +++ b/lib/libalpm/util.c @@ -1427,22 +1427,15 @@ int _alpm_fnmatch(const void *pattern, const void *string) */ void *_alpm_realloc(void **data, size_t *current, const size_t required) { - char *newdata; - - newdata = realloc(*data, required); - if(!newdata) { - _alpm_alloc_fail(required); - return NULL; - } + REALLOC(*data, required, return NULL); if (*current < required) { /* ensure all new memory is zeroed out, in both the initial * allocation and later reallocs */ - memset(newdata + *current, 0, required - *current); + memset((char*)*data + *current, 0, required - *current); } *current = required; - *data = newdata; - return newdata; + return *data; } /** This automatically grows data based on current/required. diff --git a/lib/libalpm/util.h b/lib/libalpm/util.h index 675eedec..3306a022 100644 --- a/lib/libalpm/util.h +++ b/lib/libalpm/util.h @@ -53,6 +53,7 @@ void _alpm_alloc_fail(size_t size); #define MALLOC(p, s, action) do { p = malloc(s); if(p == NULL) { _alpm_alloc_fail(s); action; } } while(0) #define CALLOC(p, l, s, action) do { p = calloc(l, s); if(p == NULL) { _alpm_alloc_fail(l * s); action; } } while(0) +#define REALLOC(p, s, action) do { void* np = realloc(p, s); if(np == NULL) { _alpm_alloc_fail(s); action; } else { p = np; } } while(0) /* This strdup macro is NULL safe- copying NULL will yield NULL */ #define STRDUP(r, s, action) do { if(s != NULL) { r = strdup(s); if(r == NULL) { _alpm_alloc_fail(strlen(s)); action; } } else { r = NULL; } } while(0) #define STRNDUP(r, s, l, action) do { if(s != NULL) { r = strndup(s, l); if(r == NULL) { _alpm_alloc_fail(l); action; } } else { r = NULL; } } while(0)
realloc can fail just like the other memory allocation functions. Add a macro to simplify handling of realloc failures, similar to the already existing MALLOC, CALLOC, etc. Replace the existing realloc uses with the new macro, allowing us to move tedious error handling to the macro. Also, in be_package and be_sync, this fixes hypothetical memory leaks (and thereafter null pointer dereferences) in case realloc fails to shrink the allocated memory. Signed-off-by: Rikard Falkeborn <rikard.falkeborn@gmail.com> --- lib/libalpm/be_local.c | 7 +------ lib/libalpm/be_package.c | 3 +-- lib/libalpm/be_sync.c | 2 +- lib/libalpm/util.c | 13 +++---------- lib/libalpm/util.h | 1 + 5 files changed, 7 insertions(+), 19 deletions(-)