On Thu, Sep 06, 2007 at 10:03:57AM +0000, Junio C Hamano wrote:
> Pierre Habouzit <madcoder@xxxxxxxxxx> writes:
>
> > diff --git a/strbuf.c b/strbuf.c
> > ...
> > +void strbuf_addvf(struct strbuf *sb, const char *fmt, va_list ap)
> > +{
> > + size_t len;
> > + va_list ap2;
> > +
> > + va_copy(ap2, ap);
> > +
> > + len = vsnprintf(sb->buf + sb->len, sb->alloc - sb->len, fmt, ap);
> > + if (len < 0) {
> > + len = 0;
> > + }
> > + if (len >= sb->alloc - sb->len) {
> > + strbuf_grow(sb, len);
> > + len = vsnprintf(sb->buf + sb->len, sb->alloc - sb->len, fmt, ap2);
> > + if (len >= sb->alloc - sb->len) {
> > + len = sb->alloc - sb->len - 1;
> > + }
> > + }
> > + sb->len = sb->len + len;
> > + sb->buf[sb->len] = '\0';
> > }
>
> Hmmmmm... Didn't we recently had a patch that used va_copy()
> which was not available somewhere and was shot down?
>
> Instead of that nice inline strbuf_addf(), you may have to do
> something like:
>
> strbuf_addf(..., fmt, ...) {
> va_list ap;
>
> va_start(ap, fmt);
> len = vsnprintf(...);
> va_end(ap);
> if (len >= sb_avail(sb)) {
> strbuf_grow(sb, len);
> va_start(ap, fmt);
> len = vsnprintf(...);
> va_end(ap);
> if (len >= sb_avail(sb))
> gaah();
> }
> sb->len += len;
> sb->buf[sb->len] = '\0';
> }
I'll do that.
> > +ssize_t strbuf_read(struct strbuf *sb, int fd)
> > +{
> > + size_t oldlen = sb->len;
> > +
> > + for (;;) {
> > + ssize_t cnt;
> > +
> > + strbuf_grow(sb, 8192);
> > + cnt = xread(fd, sb->buf + sb->len, sb->alloc - sb->len - 1);
> > + if (cnt < 0) {
> > + sb->buf[sb->len = oldlen] = '\0';
>
> Assignment inside array subscript is very hard to read.
> Besides, what's the error semantics? On error, this behaves as
> if no bytes are read (i.e. partial read results in the initial
> round is lost forever)?
Yes that is the semantics, because it's how it was used everywhere in
git: either we were able to load all the data from the file descriptor,
or weren't able to and the next thing we do is to die().
Maybe I should call the function strbuf_xread() instead ?
--
·O· Pierre Habouzit
··O madcoder@xxxxxxxxxx
OOO http://www.madism.org
Attachment:
pgp9rvW46dOkf.pgp
Description: PGP signature
