All of lore.kernel.org
 help / color / mirror / Atom feed
From: Pierre Habouzit <madcoder@debian.org>
To: Edgar Toernig <froese@gmx.de>
Cc: Junio C Hamano <gitster@pobox.com>,
	"Shawn O. Pearce" <spearce@spearce.org>,
	git@vger.kernel.org
Subject: Re: [PATCH 1/5] strbuf API additions and enhancements.
Date: Wed, 19 Sep 2007 15:36:47 +0200	[thread overview]
Message-ID: <20070919133647.GA17192@artemis.corp> (raw)
In-Reply-To: <20070919144604.7deca4f7.froese@gmx.de>

[-- Attachment #1: Type: text/plain, Size: 1603 bytes --]

On Wed, Sep 19, 2007 at 12:46:04PM +0000, Edgar Toernig wrote:
> Pierre Habouzit wrote:
> >
> > +void strbuf_addvf(struct strbuf *sb, const char *fmt, va_list ap)
> > +{
> > +	int len;
> > +
> > +	len = vsnprintf(sb->buf + sb->len, sb->alloc - sb->len, fmt, ap);
> > +	if (len < 0) {
> > +		len = 0;
> > +	}
> > +	if (len > strbuf_avail(sb)) {
> > +		strbuf_grow(sb, len);
> > +		len = vsnprintf(sb->buf + sb->len, sb->alloc - sb->len, fmt, ap);
> > +		if (len > strbuf_avail(sb)) {
> > +			die("this should not happen, your snprintf is broken");
> > +		}
> > +	}
> > +	strbuf_setlen(sb, sb->len + len);
> > +}
> 
> The second vsnprintf won't work as the first one consumed all args
> from va_list ap.  You need to va_copy the ap.  But iirc va_copy poses
> compatibility issues.  Unless va_copy is made available somehow,
> I would suggest to let the caller know that the buffer was too small
> (but isn't any more) and it has to call the function again:

  That's what I thought, and then nfvasprintf in trace.c suffers from
the same issue, as I copied the code from there.

> 	do {
> 		va_start(ap, fmt);
> 		again = strbuf_addvf(sb, fmt, ap);
> 		va_end(ap);
> 	} while (again);

  in fact doing it twice is enough but either way I don't like to impose
that to the caller :/ I mean it's totally stupid to have to do that on a
strbuf. of course we could provide a macro doing that ...
-- 
·O·  Pierre Habouzit
··O                                                madcoder@debian.org
OOO                                                http://www.madism.org

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

  reply	other threads:[~2007-09-19 13:37 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-09-18 22:39 let's refactor quoting Pierre Habouzit
2007-09-18 17:18 ` [PATCH 1/5] strbuf API additions and enhancements Pierre Habouzit
2007-09-19 12:46   ` Edgar Toernig
2007-09-19 13:36     ` Pierre Habouzit [this message]
2007-09-19 18:46       ` Junio C Hamano
2007-09-20  6:17     ` Johannes Sixt
2007-09-20  7:20       ` Kalle Olavi Niemitalo
2007-09-20 16:10         ` Jeff King
2007-09-18 20:15 ` [PATCH 2/5] sq_quote_argv and add_to_string rework with strbuf's Pierre Habouzit
2007-09-19  8:09   ` Junio C Hamano
2007-09-19  8:23     ` Pierre Habouzit
2007-09-18 21:22 ` [PATCH 3/5] Rework unquote_c_style to work on a strbuf Pierre Habouzit
2007-09-19  0:14   ` Pierre Habouzit
2007-09-19  8:09   ` Junio C Hamano
2007-09-19  8:22     ` Pierre Habouzit
2007-09-18 21:48 ` [PATCH 5/5] Avoid duplicating memory, and use xmemdupz instead of xstrdup Pierre Habouzit
2007-09-18 22:00 ` [PATCH 4/5] Full rework of quote_c_style and write_name_quoted Pierre Habouzit
2007-09-19  0:07   ` Pierre Habouzit
2007-09-19  0:55     ` Junio C Hamano
2007-09-19  1:14       ` Pierre Habouzit
2007-09-19  6:37   ` Andreas Ericsson
2007-09-19  8:00     ` Pierre Habouzit
2007-09-19  8:08       ` Andreas Ericsson
2007-09-19  8:21         ` Pierre Habouzit
2007-09-19  8:28           ` David Kastrup
2007-09-19  8:31             ` Junio C Hamano
2007-09-19  8:38               ` David Kastrup
2007-09-19  8:28   ` Junio C Hamano
2007-09-19  8:47     ` Pierre Habouzit

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=20070919133647.GA17192@artemis.corp \
    --to=madcoder@debian.org \
    --cc=froese@gmx.de \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=spearce@spearce.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.