Re: [PATCH v2 01/31] Introduce string_printf

Pedro Alves
2016-10-19T14:41:31Z

On 10/19/2016 02:53 PM, Trevor Saunders wrote:
>> +std::string
>> +string_printf (const char* fmt, ...)
>
> std::string is unfortunately kind of large for a return value
> its 4 * sizeof (void *), but maybe the simplicity matters more here.

That shouldn't matter I think? Beyond a size, ABIs will return via
the stack instead, and, also, no actual copying (in C++ sense) will
take place, because NRVO should apply here.

>
>> +{
>> + std::string str;
>> + va_list vp;
>> +
>> + /* Start by assuming some reasonable size will be sufficient. */
>> + str.resize (1024);
>> +
>> + while (1)
>> + {
>> + size_t size;
>> + int result;
>
> you could declare these at first use right?

Yeah, I considered it, and went back and forth, actually!

It'd look like this instead:

while (1)
{
va_start (vp, fmt);
size_t size = str.size ();
int result = vsnprintf (&str[0], size, fmt, vp);
va_end (vp);

str.resize (result);

if (result < size)
break;
}

Felt odd to me to see the variables declared in the middle of
va_start/va_end, since those kind of form a scope. I guess I'm
still not used to declaring PODs i the middle of blocks. If we
don't that, then when a variable is declared in the middle of a
scope it really stands out that something important might be going
on with the ctor. But maybe that's just me? I can certainly
change it.

Also, while writing this, I was also going back and forth between
doing the initial reserve:

+ str.resize (1024);

and not doing it, and instead call vsnprintf with a NULL buffer
to pre-compute the necessary size. The downsize is that
you always have to call vsnprintf twice that way. The upside
is less wasted memory, considering the case of these strings
ending up stored in some structures. I saw today that
xstrprintf (via vasprintf) uses the 'pre-compute necessary size'
instead of the 'pre-reserve' approach, so I'm leaning toward
doing that too now.

Thanks,
Pedro Alves