Re: [PATCH 1/2] Add strbuf_initf()

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

 



On Thu, 2008-03-06 at 11:55 +0100, Johannes Schindelin wrote:
> Hi,
> 
> On Thu, 6 Mar 2008, Reece Dunn wrote:
> 
> > On 06/03/2008, Mike Hommey <mh@xxxxxxxxxxxx> wrote:
> > > On Thu, Mar 06, 2008 at 02:14:43AM +0100, Johannes Schindelin wrote:
> > >  >
> > >  > The most common use of addf() was to init a strbuf and addf() right 
> > >  > away. Since it is so common, it makes sense to have a function 
> > >  > strbuf_initf() to wrap both calls into one.
> > >  >
> > >  > Unfortunately, C (and cpp) has no way to make this easy without 
> > >  > code duplication, as we need to va_init() in strbuf_addf() possibly 
> > >  > a few times.  So the code for addf() is copied.  Fortunately, the 
> > >  > code is pretty short, so not too much had to be copied as-is.
> > >
> > >
> > > The problem with code duplication is not about code size, but more 
> > > about not forgetting to fix bugs in all incarnations of the duplicated 
> > > code.
> > >
> > > Is it so ugly to use a macro ?
> > 
> > Why not have a strbuf_vaddf and strbuf_vinitf that take a va_arg as a 
> > parameter. This would mean that you don't have code duplication, and it 
> > is flexible enough if you want to add more customisations in the future. 
> > No macro needed. This is what the printf/scanf family of functions do.
> 
> The problem is that we have to restart va_list() if the buffer was too 
> small.

I think we've spent more time debating va_copy than it would take for
somebody to just lift the implementation and checks from something like
glib.  But the recent patch for vsnprintf[1] doesn't actually fix the
problem of reusing va_args in the general case; the va_list argument,
ap, is undefined after the vsnprintf() call, yet it calls it in a loop.
Just bite the bullet and pull in va_copy.  Of course, I'm just adding
to the debate here and not sending patches :/

> So your suggestion is out, unless you suggest to implement the whole 
> printf mechanism... which I hope you're not.

It's not a terrible idea, honestly.  There are several mature vsnprintf
implementations out there under friendly licenses.  We could just stick
it in compat/.  It's the only way to do reliable, cross platform
vsnprintf, in my experience.  And the issue with %I64u vs %llu could be
handle by implementing both, as Wayne Davison suggests.

Kristian

[1] Message-Id: 200803051646.13343.michal.rokos@xxxxxxxxxxx from Michal Rokos

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

  Powered by Linux