Re: [PATCH 1/1] src: out of bounds problem in smsutil

[Denis Kenzior <denkenz@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 1154 bytes --]

Hi Andreas,

>>>       }
>>>
>>>       /* Space for ranges, commas and terminator null */
>>> -    ret = g_new(char, len + nelem);
>>> +    ret = g_new0(char, len + nelem + 1);
>>
>> I'm having trouble seeing how the old code was wrong.  nelem contains
>> the number of elements.  Since the last element does not end with a
>> comma, the use of nelem + 1 in g_new is not necessary.  sprintf takes
>> care of adding the terminating null, so using g_new0 is also less
>> efficient.
>>
>> Are you adding channels that are 5 digits long by any chance?
> 
> Valgrind complains that we step outside the allocated memory by 1 byte
> since we loop the string with:
> 
> while (*topics != '\0')
> 
> the allocated memory is the size of the string and any \0 ends up
> outside. At least that's my interpretation.
> 

It might be your loop is actually going past the end, not that the
terminating NULL is not within bounds returned from
cbs_topic_ranges_to_string.  If the original code was wrong then we
should be seeing valgrind report errors on the cbs code used in
unit/test-sms.c.  I'm not seeing this at all.

Regards,
-Denis