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

From: Jeff King <peff@peff.net>
Date: Jun 19, 2014, 09:05 UTC
Message-ID: <20140619090532.GB1009@sigill.intra.peff.net>
In-Reply-To: <xmqq7g4dj1cn.fsf@gitster.dls.corp.google.com>

On Wed, Jun 18, 2014 at 03:32:08PM -0700, Junio C Hamano wrote:

Show 9 quoted lines

> >   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.

Show 7 quoted lines

> 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.

-Peff