From mboxrd@z Thu Jan  1 00:00:00 1970
From: Andrew Jones <drjones@redhat.com>
Subject: Re: [PATCH 1/3] printf: support field padding
Date: Thu, 2 Jan 2014 18:09:48 +0100
Message-ID: <20140102170948.GH9725@hawk.usersys.redhat.com>
References: <1386932286-10723-1-git-send-email-drjones@redhat.com>
 <1386932286-10723-2-git-send-email-drjones@redhat.com>
 <20131229063144.GI13601@cbox>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Cc: kvmarm@lists.cs.columbia.edu, kvm@vger.kernel.org
To: Christoffer Dall <christoffer.dall@linaro.org>
Return-path: <kvm-owner@vger.kernel.org>
Received: from mx1.redhat.com ([209.132.183.28]:44198 "EHLO mx1.redhat.com"
	rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
	id S1753557AbaABRJu (ORCPT <rfc822;kvm@vger.kernel.org>);
	Thu, 2 Jan 2014 12:09:50 -0500
Content-Disposition: inline
In-Reply-To: <20131229063144.GI13601@cbox>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

On Sat, Dec 28, 2013 at 10:31:44PM -0800, Christoffer Dall wrote:
> > +
> > +    if (npad < 0) {
> > +	char pad = props.pad;
> > +	if (pad == '0') /* ignore '0' flag with '-' flag */
> > +	    pad = ' ';
> 
> there are only the two options, so you can drop the check if
> you like.

true. removed.

> > +static int fmtnum(const char **fmt)
> > +{
> > +    const char *f = *fmt;
> > +    int len = 0, num;
> > +
> > +    if (*f == '-')
> > +	++f, ++len;
> 
> oh wow, this deserves a small comment saying that negative values are
> used to add trailing padding instead of leading.

You mean something beyond "man 3 printf; /flag"? :-)

> 
> > +
> > +    while (*f >= '0' && *f <= '9')
> > +	++f, ++len;
> > +
> > +    num = atol(*fmt);
> > +    *fmt += len;
> > +    return num;
> >  }
> 
> some funny indentation is back here...  Better check your entire patch
> for that.

The whole file has the funny indentation, I just followed suit. The
alternative is to add a patch that "fixes" all the pre-existing lib/*
files first, but for this patch I didn't think it was worth it.

drew