Re: [RFA 1/6] change gmp_string_asprintf to return an std::string

Joel Brobecker
2020-11-16T04:33:36Z

Hi Simon,

> On 2020-11-15 3:49 a.m., Joel Brobecker wrote:
> > This was suggested by Simon during a code review of this package upstream.
> > The upside is that this makes the function's API more natural and C++.
> > The downside is an extra malloc, which might be the reason why we went
> > for using a unique_xmalloc_ptr in the first place. Since this function
> > is not expected to be called frequently, the API improvement might be
> > worth the performance impact.
>
> In fact, I was thinking of doing the exact same thing as the
> string_print function. By calling vsnprintf with a NULL buffer, you can
> get how many bytes would be required to hold the result. You can then
> allocate the required buffer and call vsprintf with it.
>
> So, there's no extra allocation, although it requires formatting twice.
> But if it's good enough for string_printf (which is heavily used), it's
> good enough for gmp_string_printf.
>
> I was just wondering whether gmp_vsnprintf supported passing a NULL
> buffer like vsnprintf does, and it seems like yes. I tried the
> implementation shown below and it works (with gmp 6.2.0 at least).
>
> I would suggest renaming the function to gmp_string_printf for symmetry
> with plain string_printf.
>
> std::string
> gmp_string_asprintf (const char *fmt, ...)
> {
> va_list vp;
>
> va_start (vp, fmt);
> int size = gmp_vsnprintf (NULL, 0, fmt, vp);
> va_end (vp);
>
> std::string str (size, '\0');
>
> /* C++11 and later guarantee std::string uses contiguous memory and
> always includes the terminating '\0'. */
> va_start (vp, fmt);
> gmp_vsprintf (&str[0], fmt, vp);
> va_end (vp);
>
> return str;
> }

Thanks for the clear suggestion. We can do the above.

I don't know if that's really important in the context of this function
which I don't think will be called often, but your suggestion raises
a question: Which option should we favor? Allocating a memory region
twice, or formatting twice...

--
Joel