Subject: Re: [PATCH 1/2] strbuf: add xstrdup_fmt helper

From: René Scharfe <[email-masked]>
Date: Thu, 19 Jun 2014 18:52:39 +0200

Am 19.06.2014 11:05, schrieb Jeff King:
> On Wed, Jun 18, 2014 at 03:32:08PM -0700, Junio C Hamano wrote:
>
>>>    str = xstrdup_fmt(fmt, some, args);
>>> ---
>>> I'm open to suggestions on the name. This really is the same thing
>>> conceptually as the GNU asprintf(), but the interface is different (that
>>> function takes a pointer-to-pointer as an out-parameter, and returns the
>>> number of characters return).
>>
>> Naming it with anything "dup" certainly feels wrong.  The returned
>> string is not a duplicate of anything.
>
> I was somewhat inspired by "mkpathdup" and friends. It is quite simialr
> to mkpathdup, except that it is not limited to paths (and does not do
> any path-specific munging). I agree something with "printf" in the name
> is probably more descriptive, though.
>
>> I wonder if our callers can instead use asprintf(3) with its
>> slightly more quirky API (and then we supply compat/asprintf.c for
>> non-GNU platforms).  Right now we only have three call sites, but if
>> we anticipate that "printf-like format into an allocated piece of
>> memory" will prove be generally useful in our code base, following
>> an API that other people already have established may give our
>> developers one less thing that they have to learn.
>
> I considered that, but I do find asprintf's interface unnecessarily
> annoying (you can't return a value directly, and you run afoul of const
> issues when passing pointers to pointers). As you noted, it's not _too_
> bad, but we really get nothing from the integer return type. AFAICT, it
> helps with:
>
>    1. You know how many characters are in the string. If you cared about
>       that here, you would just use a strbuf directly, which is much more
>       flexible.
>
>    2. The error handling is different. But our x-variant would just die()
>       on errors anyway, so we do not care.
>
> So modeling after asprintf feels like carrying around unnecessary
> baggage (and I am not convinced asprintf is in wide enough use, nor that
> the function is complicated enough to care about developer familiarity).
> Part of me is tempted to call it xasprintf anyway, and use our own
> interface. GNU does not get to squat on that useful name. ;)
>
> That is probably unnecessarily confusing, though.
>
>> As usual, I would expect we would have xasprintf wrapper around it
>> to die instead of returning -1 upon allocation failure.
>
> Would it be crazy to just call it xprintf? By our current scheme that
> would be "a wrapper for printf", which means it is potentially
> confusing.
>
> Literally any other unused letter would be fine. dprintf for detach? I
> dunno.

I agree that "dup" doesn't fit and that potential callers don't need the 
length of the generated string (or should use strbuf otherwise).

Including "print" in the name is not necessary, though -- die(), error() 
and friends work fine without them.

What about simply removing the "dup_" part and using xstrfmt?

René