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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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é

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]