[pacman-dev] [PATCHv2 2/5] util: Add _alpm_realloc()

[Andrew Gregory]

On 01/28/14 at 11:12am, Florian Pritz wrote:
> Signed-off-by: Florian Pritz <bluewind at xinu.at>
> ---
> v2:
>  - Incorporated suggestions by Dave (take **data instead of *data)
>  - Handle overflows
> 
> RFC:
> systemd's greedy_realloc0() uses uint8_t instead of char for *newdata.
> Is that important or doesn't matter?
> 
> 
>  lib/libalpm/util.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++
>  lib/libalpm/util.h |  1 +
>  2 files changed, 47 insertions(+)
> 
> diff --git a/lib/libalpm/util.c b/lib/libalpm/util.c
> index 1bbc768..753cdd7 100644
> --- a/lib/libalpm/util.c
> +++ b/lib/libalpm/util.c
> @@ -1287,6 +1287,52 @@ int _alpm_fnmatch(const void *pattern, const void *string)
>  	return fnmatch(pattern, string, 0);
>  }
>  
> +/**
> + * Think of this as realloc with error handling and
> + * automatic growing based on current/required
> + *
> + * @param data source memory space
> + * @param current size of the space pointed to be data
> + * @param required size you want
> + * @return new memory
> + */

This should mention the fact that this is grow-only and "greedy" so
required is really the minimum needed and won't be the final size.
A note about possibly handing back unused portions of the allocated
memory might not hurt either.

> +void *_alpm_realloc(void **data, size_t *current, const size_t required)
> +{
> +	size_t old_size = *current;

This variable is entirely unnecessary because you don't update current
until you're about to return.

> +	char *newdata;
> +	size_t newsize = *current;
> +
> +	if(*current >= required) {
> +		return data;
> +	}
> +
> +	if(*current == 0) {
> +		newsize = ALPM_BUFFER_SIZE;
> +	} else if (required > (*current) * 2) {
> +		newsize = required * 2;
> +	} else {
> +		newsize *= 2;

I think this will read better if you set newsize = current * 2 here
instead of setting newsize = current above and doubling it.

> +	}
> +
> +	/* check for overflows */
> +	if (newsize < required) {
> +		return NULL;
> +	}
> +
> +	newdata = realloc(*data, newsize);
> +	if(!newdata) {
> +		_alpm_alloc_fail(newsize);
> +		return NULL;
> +	}
> +
> +	/* ensure all new memory is zeroed out, in both the initial
> +	 * allocation and later reallocs */
> +	memset(newdata + old_size, 0, *current - old_size);

This does nothing because *current still equals old_size.

> +	*current = newsize;
> +	*data = newdata;
> +	return newdata;
> +}
> +
>  void _alpm_alloc_fail(size_t size)
>  {
>  	fprintf(stderr, "alloc failure: could not allocate %zd bytes\n", size);
> diff --git a/lib/libalpm/util.h b/lib/libalpm/util.h
> index b566868..031254a 100644
> --- a/lib/libalpm/util.h
> +++ b/lib/libalpm/util.h
> @@ -141,6 +141,7 @@ int _alpm_raw_ncmp(const char *first, const char *second, size_t max);
>  int _alpm_access(alpm_handle_t *handle, const char *dir, const char *file, int amode);
>  int _alpm_fnmatch_patterns(alpm_list_t *patterns, const char *string);
>  int _alpm_fnmatch(const void *pattern, const void *string);
> +void *_alpm_realloc(void **data, size_t *current, const size_t required);
>  
>  #ifndef HAVE_STRSEP
>  char *strsep(char **, const char *);
> -- 
> 1.8.5.3