Re: [PATCH] Rework strbuf API and semantics.

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

 



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';
	}


> +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)?
-
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