Re: [RFC] Time to talk about StringNG merge again?

On 07/31/2013 06:18 AM, Kinkie wrote:
> On Wed, Jul 31, 2013 at 2:53 AM, Alex Rousskov wrote:
>> On 07/30/2013 03:56 AM, Kinkie wrote:
>>> len_ += sz;
>>> // TODO: this does NOT belong here, but to class-init or autoconf
>>> /* on Linux and FreeBSD, '\0' is not counted in return value */
>>> /* on XXX it might be counted */
>>> /* check that '\0' is appended and not counted */
>>>
>>> if (operator[](len_-1) == '\0') {
>>> --sz;
>>> --len_;
>>> }
>>
>> There is no guarantee that len_ is positive. We could start with a zero
>> len_ and an empty "" pattern would result in zero sz. The above code may
>> then access character at -1 offset of the raw storage array...

> Ok.
> This will go away by itself when we move to unsigned size_type.

I do not see how it will go away. If an unsigned len_ is zero, you would
be accessing a non-existent byte at a very large (unsigned)-1 offset,
which may even be worse than accessing a non-existent byte at -1 offset.

Another small bug in that code is that arguments or even fmt might
actually end with a valid \0 byte that should be appended and counted
(not a c-string terminating byte).

This check should be done once, with controlled fmt and arguments so
that we can detect problems reliably. You can do that at module
initialization time, for example (./configure might be too early because
libraries might change) and set a static SnprintfCountsTerminator
variable accordingly.

char buf[16];
SprintfCountsTerminator = snprintf(buf, sizeof(buf), "%s", "1") == 2;

Cheers,

Alex.
Received on Wed Jul 31 2013 - 17:46:05 MDT