Re: [PATCH] APEI/ERST: Fix error message formatting

[Naveen N. Rao]

On 07/25/2013 11:02 PM, Bjorn Helgaas wrote:
> On Thu, Jul 25, 2013 at 5:23 AM, Naveen N. Rao
> <naveen.n.rao@xxxxxxxxxxxxxxxxxx> wrote:
> > On 07/24/2013 10:53 PM, Joe Perches wrote:
> > > On Wed, 2013-07-24 at 22:43 +0530, Naveen N. Rao wrote:
> > > > On 2013/07/22 11:01PM, Borislav Petkov wrote:
> > > > > From: Borislav Petkov <bp@xxxxxxx>
> > > > >
> > > > > [    5.525861] ERST: Can not request iomem region <0x        c7eff000-0x
> > > > > c7f00000> for ERST.
> > > > >
> > > > > This needs to have leading zeroes. Make it so.
> > >
> > >
> > > Why does it need leading zeros?
> > >
> > > > While looking at this, I noticed that we seem to be using varying field
> > > > widths in our APEI code:
> > > > - einj.c has two instances using %#010llx.
> > > > - apei-base.c uses widths of 10 (4 bytes) and 6 (2 bytes).
> > > >
> > > > Not sure if these are intentional and those fields truly aren't 64-bit
> > > > (as suggested by the usage of long long int).
> > >
> > >
> > > I suggest using "0x%llx" everywhere unless there's a
> > > compelling reason like columnar alignment for them.
> >
> >
> > I think that might be better. I see that these changes were done in commit
> > 46b91e37. Copying Bjorn Helgaas.
>
> As the 46b91e37 changelog says, it was done to use "the normal
> %pR-like format".  I think that's a valid goal.  When we're printing
> the same sort of information, we should use the same sort of format.

Bjorn,
My key question was about why we are using a field width of 10 implying 
a 32-bit value, rather than a field width of 18 as suggested by the data 
type? This shouldn't truncate the value, but if we are specifying the 
field width for alignment, seems to me it is better to match the data type.

Thanks,
Naveen

> But I don't think the "Can not request iomem region <0x
> c7eff000-0x        c7f00000> for ERST" output mentioned in the
> original post was affected by 46b91e37.  I would suggest a change
> similar to 46b91e37 for ERST, and I would suggest using the leading
> zeros, with %#010llx for physical memory addresses and %#06llx for
> ioport addresses.  That's what %pR uses, and it produces columnar
> alignment in many cases (though not this one).
>
> Bjorn

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/