| Message ID | 1397130265-2389-1-git-send-email-julien.grall@linaro.org |
|---|---|
| State | Accepted, archived |
| Headers | show |
On 10/04/14 12:44, Julien Grall wrote: > The syndrome value is printed in hexadecimal. Prefix it by 0x for less > confusion. > > Signed-off-by: Julien Grall <julien.grall@linaro.org> Xen is trying to converge on '%#' rather than an explicit 0x. Also, it looks as if you want to do the same for IL ~Andrew > --- > xen/arch/arm/traps.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c > index 9eeed92..858abe5 100644 > --- a/xen/arch/arm/traps.c > +++ b/xen/arch/arm/traps.c > @@ -1616,7 +1616,7 @@ asmlinkage void do_trap_hypervisor(struct cpu_user_regs *regs) > break; > default: > bad_trap: > - printk("Hypervisor Trap. HSR=0x%x EC=0x%x IL=%x Syndrome=%"PRIx32"\n", > + printk("Hypervisor Trap. HSR=0x%x EC=0x%x IL=%x Syndrome=0x%"PRIx32"\n", > hsr.bits, hsr.ec, hsr.len, hsr.iss); > do_unexpected_trap("Hypervisor", regs); > }
On 04/10/2014 12:53 PM, Andrew Cooper wrote: > On 10/04/14 12:44, Julien Grall wrote: >> The syndrome value is printed in hexadecimal. Prefix it by 0x for less >> confusion. >> >> Signed-off-by: Julien Grall <julien.grall@linaro.org> > > Xen is trying to converge on '%#' rather than an explicit 0x. I use to prefer explicit 0x :). Anyway, I will change to use %#. > > Also, it looks as if you want to do the same for IL Oh right, I forgot this one. I will send a new version of the patch. Regards,
On Thu, 2014-04-10 at 12:53 +0100, Andrew Cooper wrote: > On 10/04/14 12:44, Julien Grall wrote: > > The syndrome value is printed in hexadecimal. Prefix it by 0x for less > > confusion. > > > > Signed-off-by: Julien Grall <julien.grall@linaro.org> > > Xen is trying to converge on '%#' rather than an explicit 0x. Really? I've been moving in the opposite direction because e.g. %010#lx does the wrong thing with zero (0000000000 instead of 0x00000000). Not sure about plain %#x though... > Also, it looks as if you want to do the same for IL IL must be 2 or 4, for clarity it would be better as %d rather than 0x%x I think. > > ~Andrew > > > --- > > xen/arch/arm/traps.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c > > index 9eeed92..858abe5 100644 > > --- a/xen/arch/arm/traps.c > > +++ b/xen/arch/arm/traps.c > > @@ -1616,7 +1616,7 @@ asmlinkage void do_trap_hypervisor(struct cpu_user_regs *regs) > > break; > > default: > > bad_trap: > > - printk("Hypervisor Trap. HSR=0x%x EC=0x%x IL=%x Syndrome=%"PRIx32"\n", > > + printk("Hypervisor Trap. HSR=0x%x EC=0x%x IL=%x Syndrome=0x%"PRIx32"\n", > > hsr.bits, hsr.ec, hsr.len, hsr.iss); > > do_unexpected_trap("Hypervisor", regs); > > } >
On 10/04/14 13:01, Ian Campbell wrote: > On Thu, 2014-04-10 at 12:53 +0100, Andrew Cooper wrote: >> On 10/04/14 12:44, Julien Grall wrote: >>> The syndrome value is printed in hexadecimal. Prefix it by 0x for less >>> confusion. >>> >>> Signed-off-by: Julien Grall <julien.grall@linaro.org> >> Xen is trying to converge on '%#' rather than an explicit 0x. > Really? I've been moving in the opposite direction because e.g. %010#lx > does the wrong thing with zero (0000000000 instead of 0x00000000). > > Not sure about plain %#x though... %#x DoesTheRightThing, and is a character shorter than its alternative. Jan appears to have replaced most of the x86 examples. I also get slightly frustrated with Format strings with an explicit width do the specified thing (limit the length of the entire formatted entry), rather than the useful thing (sticking '0x' before a number with an explicit width). By the time you have got a zero-extended explicitly-width'd format string, the 0x is hardly the end of the world > >> Also, it looks as if you want to do the same for IL > IL must be 2 or 4, for clarity it would be better as %d rather than 0x%x > I think. Agreed ~Andrew
On Thu, 2014-04-10 at 12:44 +0100, Julien Grall wrote: > The syndrome value is printed in hexadecimal. Prefix it by 0x for less > confusion. > > Signed-off-by: Julien Grall <julien.grall@linaro.org> I've acked + applied this. Irrespective of any apparent move to prefer % #x this is an improvement. > --- > xen/arch/arm/traps.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c > index 9eeed92..858abe5 100644 > --- a/xen/arch/arm/traps.c > +++ b/xen/arch/arm/traps.c > @@ -1616,7 +1616,7 @@ asmlinkage void do_trap_hypervisor(struct cpu_user_regs *regs) > break; > default: > bad_trap: > - printk("Hypervisor Trap. HSR=0x%x EC=0x%x IL=%x Syndrome=%"PRIx32"\n", > + printk("Hypervisor Trap. HSR=0x%x EC=0x%x IL=%x Syndrome=0x%"PRIx32"\n", > hsr.bits, hsr.ec, hsr.len, hsr.iss); > do_unexpected_trap("Hypervisor", regs); > }
On 05/02/2014 01:56 PM, Ian Campbell wrote: > On Thu, 2014-04-10 at 12:44 +0100, Julien Grall wrote: >> The syndrome value is printed in hexadecimal. Prefix it by 0x for less >> confusion. >> >> Signed-off-by: Julien Grall <julien.grall@linaro.org> > > I've acked + applied this. Irrespective of any apparent move to prefer % > #x this is an improvement. Thanks. So what was the conclusion about % vs #x. Which one shall we use? Regards,
On Fri, 2014-05-02 at 14:19 +0100, Julien Grall wrote: > On 05/02/2014 01:56 PM, Ian Campbell wrote: > > On Thu, 2014-04-10 at 12:44 +0100, Julien Grall wrote: > >> The syndrome value is printed in hexadecimal. Prefix it by 0x for less > >> confusion. > >> > >> Signed-off-by: Julien Grall <julien.grall@linaro.org> > > > > I've acked + applied this. Irrespective of any apparent move to prefer % > > #x this is an improvement. > > Thanks. So what was the conclusion about % vs #x. Which one shall we use? I'm perfectly fine with 0x%x for ARM code. I think Andy's comment about "Xen moving towards" was overstating things somewhat. Ian.
On 05/02/2014 02:21 PM, Ian Campbell wrote: > On Fri, 2014-05-02 at 14:19 +0100, Julien Grall wrote: >> On 05/02/2014 01:56 PM, Ian Campbell wrote: >>> On Thu, 2014-04-10 at 12:44 +0100, Julien Grall wrote: >>>> The syndrome value is printed in hexadecimal. Prefix it by 0x for less >>>> confusion. >>>> >>>> Signed-off-by: Julien Grall <julien.grall@linaro.org> >>> >>> I've acked + applied this. Irrespective of any apparent move to prefer % >>> #x this is an improvement. >> >> Thanks. So what was the conclusion about % vs #x. Which one shall we use? > > I'm perfectly fine with 0x%x for ARM code. I think Andy's comment about > "Xen moving towards" was overstating things somewhat. Ok. FIY, Jan Beulich asked me to change some 0x%x into #x in common code when I sent patch a couple of month ago.
>>> On 02.05.14 at 15:25, <julien.grall@linaro.org> wrote: > On 05/02/2014 02:21 PM, Ian Campbell wrote: >> On Fri, 2014-05-02 at 14:19 +0100, Julien Grall wrote: >>> On 05/02/2014 01:56 PM, Ian Campbell wrote: >>>> On Thu, 2014-04-10 at 12:44 +0100, Julien Grall wrote: >>>>> The syndrome value is printed in hexadecimal. Prefix it by 0x for less >>>>> confusion. >>>>> >>>>> Signed-off-by: Julien Grall <julien.grall@linaro.org> >>>> >>>> I've acked + applied this. Irrespective of any apparent move to prefer % >>>> #x this is an improvement. >>> >>> Thanks. So what was the conclusion about % vs #x. Which one shall we use? >> >> I'm perfectly fine with 0x%x for ARM code. I think Andy's comment about >> "Xen moving towards" was overstating things somewhat. > > Ok. FIY, Jan Beulich asked me to change some 0x%x into #x in common code > when I sent patch a couple of month ago. I can only repeat that: The format string is one byte shorter with the %#x form, and since this accumulates and we shouldn't be making the hypervisor image bigger than it needs to be, I think we should prefer that form. But of course - as everywhere - the maintainer(s) of a particular piece of code has/have the final say. Jan
On Fri, 2014-05-02 at 15:08 +0100, Jan Beulich wrote: > >>> On 02.05.14 at 15:25, <julien.grall@linaro.org> wrote: > > On 05/02/2014 02:21 PM, Ian Campbell wrote: > >> On Fri, 2014-05-02 at 14:19 +0100, Julien Grall wrote: > >>> On 05/02/2014 01:56 PM, Ian Campbell wrote: > >>>> On Thu, 2014-04-10 at 12:44 +0100, Julien Grall wrote: > >>>>> The syndrome value is printed in hexadecimal. Prefix it by 0x for less > >>>>> confusion. > >>>>> > >>>>> Signed-off-by: Julien Grall <julien.grall@linaro.org> > >>>> > >>>> I've acked + applied this. Irrespective of any apparent move to prefer % > >>>> #x this is an improvement. > >>> > >>> Thanks. So what was the conclusion about % vs #x. Which one shall we use? > >> > >> I'm perfectly fine with 0x%x for ARM code. I think Andy's comment about > >> "Xen moving towards" was overstating things somewhat. > > > > Ok. FIY, Jan Beulich asked me to change some 0x%x into #x in common code > > when I sent patch a couple of month ago. > > I can only repeat that: The format string is one byte shorter with > the %#x form, and since this accumulates and we shouldn't be > making the hypervisor image bigger than it needs to be, I think we > should prefer that form. We'd need > 2000 of these before there was an even chance of pushing us into the next page. Is this really the lowest hanging fruit in terms of hypervisor size? > But of course - as everywhere - the > maintainer(s) of a particular piece of code has/have the final say. TBH it's purely cosmetic, for values which one expects to see in hex I prefer to see 0x0 to 0, especially in e.g. a list of 0 and non-0 items. Ian.
diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c index 9eeed92..858abe5 100644 --- a/xen/arch/arm/traps.c +++ b/xen/arch/arm/traps.c @@ -1616,7 +1616,7 @@ asmlinkage void do_trap_hypervisor(struct cpu_user_regs *regs) break; default: bad_trap: - printk("Hypervisor Trap. HSR=0x%x EC=0x%x IL=%x Syndrome=%"PRIx32"\n", + printk("Hypervisor Trap. HSR=0x%x EC=0x%x IL=%x Syndrome=0x%"PRIx32"\n", hsr.bits, hsr.ec, hsr.len, hsr.iss); do_unexpected_trap("Hypervisor", regs); }
The syndrome value is printed in hexadecimal. Prefix it by 0x for less confusion. Signed-off-by: Julien Grall <julien.grall@linaro.org> --- xen/arch/arm/traps.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)