[PATCH] (backport) fix usage of deprecated D-Bus functions

[Jouni Malinen]

On Tue, Mar 04, 2008 at 09:58:40AM -0500, Dan Williams wrote:

> The purpose of the patch is to get rid of
> dbus_message_iter_get_array_len(), which is deprecated because it
> doesn't work everywhere or the way we expect it.  Therefore, instead of
> pre-sizing the arrays, we start with a reasonable size and grow the
> arrays when needed while demarshalling the D-Bus message.

Thanks! This looks otherwise fine, but the use of realloc looks a bit
suspect. I did not go through all the possible cleanup paths, but it
does not look like the old buffer would be freed in case of an
allocation error (i.e., a corner case memory leak). Did I miss something
or should this be fixed to make sure that the old buffer gets freed?

> +	entry->bytearray_value = buffer;

> +			buffer = realloc(buffer, BYTE_ARRAY_ITEM_SIZE * (count + BYTE_ARRAY_CHUNK_SIZE));
> +			if (buffer == NULL) {
> +				perror("_wpa_dbus_dict_entry_get_byte_array["
> +				       "dbus] out of memory trying to "
> +				       "retrieve the string array");
> +				goto done;
> +			}

Where will the old buffer (buffer prior to that realloc call) be freed?
entry->bytearray_value has the buffer here, but it was not immediately
obvious whether that is freed somewhere.

> -			char **tmp;
> -			tmp = realloc(buffer, sizeof(char *) * (count + 8));
> -			if (tmp == NULL) {
> +		if ((count % STR_ARRAY_CHUNK_SIZE) == 0 && count != 0) {
> +			buffer = realloc(buffer, STR_ARRAY_ITEM_SIZE * (count + STR_ARRAY_CHUNK_SIZE));
> +			if (buffer == NULL) {
>  				perror("_wpa_dbus_dict_entry_get_string_array["
>  				       "dbus] out of memory trying to "
>  				       "retrieve the string array");
> -				free(buffer);
> -				buffer = NULL;
>  				goto done;

Same here.. The old version was explicitly taking care of the error case
and freeing the old buffer on realloc failure. The new version leaves a
pointer to the old buffer into entry->strarray_value, but it was not
clear whether that gets freed on error path.

Are both of these freed in wpa_dbus_dict_entry_clear()? Is that function
guaranteed to be called for all error cases?

-- 
Jouni Malinen                                            PGP id EFC895FA