Code review comment for lp:~xnox/whoopsie/fix-realloc

Revision history for this message
Dimitri John Ledkov (xnox) wrote :

On 10 June 2014 09:31, John Lenton <email address hidden> wrote:
> Review: Disapprove
>
> The bug is real (it's a bug in the tests), the fix is wrong.
>
> What needs doing is using a g_malloc'ed string in the tests, so that it's the same flavour of memory in both places.

Well, the type accepted is char **, that is an array of pointers of
pointers to chars.
It's not const, thus the function may mutate the pointers as it likes,
within reason.
In practice it only mutates it, if it decides to append values to it.
A pointer to string literal is "const char *" but there is no easy way
to detect "const" when dereferencing "char **", thus imho
implementation should be resilient to dealing with string literals as
we can't compile time enforce them.

Ideally one would use str_array as e.g. provide by more mature libinh
http://libnih.la/nih-String.html#nih-str-array-new library, all
strings there are are always reallocated since it's always valid to
append a string literal to the string array.

Why does imei function mutate first string in the array, instead of
prepending/appending another value to the array?

--
Regards,

Dimitri.

« Back to merge proposal