Re: Generic C++ templates/library for FreeBSD base

On 3/14/25 00:19, Gleb Popov wrote:
> On Thu, Mar 13, 2025 at 11:53 PM John Baldwin <jhb@freebsd.org> wrote:
>>
>> One is a stringf() function that accepts printf() style format string and
>> arguments and returns a std::string.  I know C++23 adds <format>, but we
>> can't assume that yet, and this function is probably more useful when
>> adapting existing C code.  Compared to some other solutions, I chose to
>> wrap asprintf() and do an extra copy at the end into a std::string rather
>> than calling vsnprintf() twice.  It seems less ugly than the vsnprintf()
>> solutions also:
>>
>> https://github.com/freebsd/freebsd-src/commit/01bd3d89ddf9ccbf884e52fe7289e8a9278e2d63
> 
> I wonder why std::string always copies the data passed into the constructor.
> 
> It is possible to avoid extra alloc by returning a std::string_view,
> but this would force the caller into freeing the memory manually.
> Maybe derive from it and extend the destructor, but this just brings
> me to the initial question.

What you would want is something where you could do std::move() of the pointer
returned by asprintf(), into the std::string object, but that assumes that
new/delete are always using malloc/free (which is probably true in practice).

The other variants of this I've seen avoid the copy by calling vsnprintf twice,
but I suspect the extra malloc + memcpy is cheaper than parsing the format string
and walking a va_list twice.  If someone really cared, perhaps this could be
reimplemented using funopen() where you construct an empty std::string and pass
that as the cookie and have the backend methods grow the string and copy the
bytes into the formatted copy.  However, I'm not sure that this matters enough
performance-wise to justify the complexity.  (I also find the vsnprintf versions
more complex.)  In ctld, for example, this ends up getting used for logging
config file errors which is not a performance-critical path.

>> The other is a FILE_up that wraps a FILE * and uses fclose() as its deleter
>> buried as part of a larger change:
>>
>> https://github.com/freebsd/freebsd-src/commit/77b800022b18ab61983b22022b16943355b2bf75
> 
> I see the code uses exceptions. Was it a conscientious decision?

new throws std::bad_alloc and this is trying to catch those and fail to
load a new config (e.g. when getting a SIGHUP).

That said, right now the uclparse code has to propagate "return (false)" up
through 3 or 4 layers (and I've already had to fix several places that didn't
handle it correctly).  I'm considering turning parse errors in the UCL side
at least into exceptions because it would simplify the code a fair bit.

-- 
John Baldwin