From: Jeff King <peff@peff.net>
Date: Wed, 24 Oct 2012 08:25:12 GMT
Subject: Re: [PATCH 7/9] pretty: support padding placeholders, %< %> and %><
Message-ID: <20121024082512.GA22127@sigill.intra.peff.net>
In-Reply-To: <1348391433-11300-8-git-send-email-pclouds@gmail.com>

On Sun, Sep 23, 2012 at 04:10:31PM +0700, Nguyen Thai Ngoc Duy wrote:

> +	else {
> +		int sb_len = sb->len, offset;
> +		switch (c->flush_type) {
> +		case flush_left:
> +			offset = padding - len;
> +			break;
> +		case flush_right:
> +			offset = 0;
> +			break;
> +		case flush_both:
> +			offset = (padding - len) / 2;
> +			break;
> +		case no_flush: /* to make gcc happy */
> +			break;
> +		}
> +		/*
> +		 * we calculate padding in columns, now
> +		 * convert it back to chars
> +		 */
> +		padding = padding - len + local_sb.len;
> +		strbuf_grow(sb, padding);
> +		strbuf_setlen(sb, sb_len + padding);
> +		memset(sb->buf + sb_len, ' ', sb->len - sb_len);
> +		memcpy(sb->buf + sb_len + offset, local_sb.buf,
> +		       local_sb.len);
> +	}

gcc complains (rightly, I think) that offset can be used uninitialized
in the final line (looks like it is from the no_flush case). If it is a
"can never happen" case that is there to appease gcc in the switch
statement, should we drop a die("BUG: XXX") there? If so, what would the
XXX be?

-Peff