From: "René Scharfe" <l.s.r@web.de>
To: Jeff King <peff@peff.net>, Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCH 1/2] strbuf: add xstrdup_fmt helper
Date: Thu, 19 Jun 2014 18:52:39 +0200 [thread overview]
Message-ID: <53A31557.8070901@web.de> (raw)
In-Reply-To: <20140619090532.GB1009@sigill.intra.peff.net>
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é
next prev parent reply other threads:[~2014-06-19 16:53 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-06-18 20:00 [PATCH 0/2] dropping manual malloc calculations Jeff King
2014-06-18 20:01 ` [PATCH 1/2] strbuf: add xstrdup_fmt helper Jeff King
2014-06-18 22:32 ` Junio C Hamano
2014-06-19 9:05 ` Jeff King
2014-06-19 16:49 ` Junio C Hamano
2014-06-19 21:16 ` [PATCH v2] dropping manual malloc calculations Jeff King
2014-06-19 21:18 ` [PATCH v2 01/10] strbuf: add xstrfmt helper Jeff King
2014-06-19 21:19 ` [PATCH v2 02/10] use xstrfmt in favor of manual size calculations Jeff King
2014-06-19 21:19 ` [PATCH v2 03/10] use xstrdup instead of xmalloc + strcpy Jeff King
2014-06-19 21:24 ` [PATCH v2 04/10] use xstrfmt to replace xmalloc + sprintf Jeff King
2014-06-19 21:26 ` [PATCH v2 05/10] use xstrfmt to replace xmalloc + strcpy/strcat Jeff King
2014-06-19 21:28 ` [PATCH v2 06/10] setup_git_env: use git_pathdup instead of xmalloc + sprintf Jeff King
2014-06-23 10:21 ` Eric Sunshine
2014-06-23 22:43 ` Junio C Hamano
2014-06-24 13:02 ` Duy Nguyen
2014-06-24 13:30 ` Duy Nguyen
2014-06-24 20:58 ` Jeff King
2014-06-25 12:37 ` Duy Nguyen
2014-06-25 17:20 ` Junio C Hamano
2014-06-25 17:22 ` Jeff King
2014-06-25 19:54 ` Junio C Hamano
2014-06-19 21:28 ` [PATCH v2 07/10] sequencer: use argv_array_pushf Jeff King
2014-06-19 21:29 ` [PATCH v2 08/10] merge: use argv_array when spawning merge strategy Jeff King
2014-06-19 21:29 ` [PATCH v2 09/10] walker_fetch: fix minor memory leak Jeff King
2014-06-19 21:30 ` [PATCH v2 10/10] unique_path: fix unlikely heap overflow Jeff King
2014-06-19 16:52 ` René Scharfe [this message]
2014-06-18 20:02 ` [PATCH 2/2] use xstrdup_fmt in favor of manual size calculations Jeff King
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=53A31557.8070901@web.de \
--to=l.s.r@web.de \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=peff@peff.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).