[pacman-dev] [PATCH] Add REALLOC macro to simplify realloc error handling

[Allan McRae]

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 at 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)
>