From: Junio C Hamano <gitster@pobox.com>
To: Miklos Vajna <vmiklos@frugalware.org>
Cc: git@vger.kernel.org,
Johannes Schindelin <Johannes.Schindelin@gmx.de>,
Olivier Marin <dkr@freesurf.fr>
Subject: Re: [PATCH 11/15] Add strbuf_vaddf(), use it in strbuf_addf(), and add strbuf_initf()
Date: Fri, 27 Jun 2008 19:00:53 -0700 [thread overview]
Message-ID: <7vk5gal3my.fsf@gitster.siamese.dyndns.org> (raw)
In-Reply-To: 7ea320cd49601de0e331777f82528876101fd946.1214581610.git.vmiklos@frugalware.org
Do you really think these two patches belong to the series, I seriously
have to wonder?
The series uses strbuf_initf() from 7 call sites to save a few lines per
each call site, and for that you have a 140-line patch to strbuf.c (in
aggregate between this one and the "Oops, I need to support this, too" one
after this).
This new code that is not essential to the series need to be carefully
vetted to avoid risks of potentially affecting existing 70+ users of
strbuf_addf(), which used to use (hopefully more trustworthy) vsnprintf()
from the system, but now uses the new and unproven strbuf_vaddf() that
has "Only supports %<these>" disclaimer at the top.
I am not saying the strbuf_vaddf() patch is not worth considering. It
might help platforms without a working vsnprintf(). But its merit needs
to be defended separately, and I do not want "merge in C" series to be
taken hostage by it.
Other than that, I did not see anything obviously wrong in the diff
between the previous round and this series.
Thanks.
next prev parent reply other threads:[~2008-06-28 2:02 UTC|newest]
Thread overview: 81+ messages / expand[flat|nested] mbox.gz Atom feed top
2008-06-27 16:21 [PATCH 00/15] Build in merge Miklos Vajna
2008-06-27 16:21 ` [PATCH 01/15] Move split_cmdline() to alias.c Miklos Vajna
2008-06-27 16:21 ` [PATCH 02/15] Move commit_list_count() to commit.c Miklos Vajna
2008-06-27 16:21 ` [PATCH 03/15] Move parse-options's skip_prefix() to git-compat-util.h Miklos Vajna
2008-06-27 16:21 ` [PATCH 04/15] Add new test to ensure git-merge handles pull.twohead and pull.octopus Miklos Vajna
2008-06-27 16:21 ` [PATCH 05/15] Move read_cache_unmerged() to read-cache.c Miklos Vajna
2008-06-27 16:21 ` [PATCH 06/15] git-fmt-merge-msg: make it usable from other builtins Miklos Vajna
2008-06-27 16:22 ` [PATCH 07/15] Introduce get_octopus_merge_bases() in commit.c Miklos Vajna
2008-06-27 16:22 ` [PATCH 08/15] Add new test to ensure git-merge handles more than 25 refs Miklos Vajna
2008-06-27 16:22 ` [PATCH 09/15] Introduce get_merge_bases_many() Miklos Vajna
2008-06-27 16:22 ` [PATCH 10/15] Introduce reduce_heads() Miklos Vajna
2008-06-27 16:22 ` [PATCH 11/15] Add strbuf_vaddf(), use it in strbuf_addf(), and add strbuf_initf() Miklos Vajna
2008-06-27 16:22 ` [PATCH 12/15] strbuf_vaddf(): support %*s, too Miklos Vajna
2008-06-27 16:22 ` [PATCH 13/15] Add new test case to ensure git-merge reduces octopus parents when possible Miklos Vajna
2008-06-27 16:22 ` [PATCH 14/15] Add new test case to ensure git-merge prepends the custom merge message Miklos Vajna
2008-06-27 16:22 ` [PATCH 15/15] Build in merge Miklos Vajna
2008-06-27 17:09 ` Miklos Vajna
2008-06-28 2:00 ` Junio C Hamano [this message]
2008-06-28 2:33 ` [PATCH 11/15] Add strbuf_vaddf(), use it in strbuf_addf(), and add strbuf_initf() Miklos Vajna
2008-06-28 2:38 ` [PATCH 13/13] Build in merge Miklos Vajna
2008-06-29 7:46 ` Junio C Hamano
2008-06-29 8:11 ` Jakub Narebski
2008-06-30 1:36 ` Miklos Vajna
2008-06-30 1:39 ` Miklos Vajna
2008-06-30 5:44 ` Junio C Hamano
2008-06-30 17:41 ` Alex Riesen
2008-07-01 2:13 ` Miklos Vajna
2008-07-01 2:22 ` [PATCH 13/14] git-commit-tree: make it usable from other builtins Miklos Vajna
2008-07-01 5:07 ` Johannes Schindelin
2008-07-01 5:50 ` Junio C Hamano
2008-07-01 12:09 ` Miklos Vajna
2008-07-01 2:22 ` [PATCH 14/14] Build in merge Miklos Vajna
2008-07-01 2:37 ` [PATCH 00/14] " Miklos Vajna
2008-07-01 2:37 ` [PATCH 08/14] Add new test to ensure git-merge handles more than 25 refs Miklos Vajna
2008-07-01 2:37 ` [PATCH 13/14] git-commit-tree: make it usable from other builtins Miklos Vajna
2008-07-01 2:37 ` [PATCH 14/14] Build in merge Miklos Vajna
2008-07-01 6:23 ` Junio C Hamano
2008-07-01 12:50 ` Miklos Vajna
2008-07-01 13:18 ` Miklos Vajna
2008-07-01 23:55 ` Junio C Hamano
2008-07-02 7:43 ` Miklos Vajna
2008-07-06 8:50 ` Junio C Hamano
2008-07-06 9:43 ` Junio C Hamano
2008-07-07 17:17 ` Miklos Vajna
2008-07-07 18:15 ` Junio C Hamano
2008-07-07 23:42 ` [PATCH] " Miklos Vajna
2008-07-08 0:32 ` Junio C Hamano
2008-07-08 0:53 ` Junio C Hamano
2008-07-08 1:18 ` Miklos Vajna
2008-07-08 1:00 ` Miklos Vajna
2008-07-08 1:05 ` Junio C Hamano
2008-07-08 1:41 ` Miklos Vajna
2008-07-06 12:38 ` [PATCH 14/14] " Johannes Schindelin
2008-07-06 19:39 ` Junio C Hamano
2008-07-07 17:24 ` [PATCH] " Miklos Vajna
2008-07-07 17:35 ` Miklos Vajna
2008-07-01 7:27 ` [PATCH 14/14] " Junio C Hamano
2008-07-01 12:55 ` Miklos Vajna
2008-06-30 22:44 ` [PATCH 13/13] " Olivier Marin
2008-06-30 22:58 ` Miklos Vajna
2008-06-30 5:40 ` Junio C Hamano
2008-06-30 22:48 ` Miklos Vajna
2008-06-28 17:33 ` [PATCH 11/15] Add strbuf_vaddf(), use it in strbuf_addf(), and add strbuf_initf() Johannes Schindelin
2008-06-29 8:07 ` Junio C Hamano
2008-06-29 13:40 ` Johannes Schindelin
2008-06-29 20:17 ` Alex Riesen
2008-06-29 20:24 ` Junio C Hamano
2008-06-29 20:30 ` Sverre Rabbelier
2008-06-27 17:06 ` [PATCH 08/15] Add new test to ensure git-merge handles more than 25 refs Miklos Vajna
2008-06-29 13:30 ` [PATCH 04/15] Add new test to ensure git-merge handles pull.twohead and pull.octopus Olivier Marin
2008-06-29 14:51 ` [PATCH] " Miklos Vajna
2008-06-29 15:11 ` Miklos Vajna
2008-07-04 16:34 ` [PATCH 04/15] " Mike Ralphson
2008-07-05 0:26 ` Miklos Vajna
2008-07-05 0:32 ` [PATCH] Fix t7601-merge-pull-config.sh on AIX Miklos Vajna
2008-07-05 1:49 ` Junio C Hamano
2008-06-29 14:05 ` [PATCH 01/15] Move split_cmdline() to alias.c Olivier Marin
2008-06-29 14:15 ` Johannes Schindelin
2008-06-29 14:29 ` Olivier Marin
2008-06-29 14:43 ` Johannes Schindelin
2008-06-30 22:51 ` Olivier Marin
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=7vk5gal3my.fsf@gitster.siamese.dyndns.org \
--to=gitster@pobox.com \
--cc=Johannes.Schindelin@gmx.de \
--cc=dkr@freesurf.fr \
--cc=git@vger.kernel.org \
--cc=vmiklos@frugalware.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 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).