| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Sun, 27 Jan 2008 22:51:00 +0100
From: "Paolo Ciarrocchi" <paolo.ciarrocchi@...il.com>
To: "Dmitri Vorobiev" <dmitri.vorobiev@...il.com>
Cc: "Linux Kernel" <linux-kernel@...r.kernel.org>,
"Ingo Molnar" <mingo@...e.hu>
Subject: Re: [PATCH] X86: coding style fixes to arch/x86/math-errors.c
On Jan 27, 2008 10:45 PM, Dmitri Vorobiev <dmitri.vorobiev@...il.com> wrote:
> > - math_abort(FPU_info,SIGILL);
> > + math_abort(FPU_info , SIGILL);
>
> Looks like you're trading bad for worse here.
Should I make it read like math_abort(FPU_info, SIGILL); ?
>
> > }
> >
> >
> > @@ -88,61 +86,69 @@ void FPU_printall(void)
> > {
> > int i;
> > static const char *tag_desc[] = { "Valid", "Zero", "ERROR", "Empty",
> > - "DeNorm", "Inf", "NaN" };
> > + "DeNorm", "Inf", "NaN" };
> > u_char byte1, FPU_modrm;
> > unsigned long address = FPU_ORIG_EIP;
> >
> > RE_ENTRANT_CHECK_OFF;
> > /* No need to check access_ok(), we have previously fetched these bytes. */
> > printk("At %p:", (void *) address);
> > - if ( FPU_CS == __USER_CS )
> > - {
> > + if (FPU_CS == __USER_CS) {
> > #define MAX_PRINTED_BYTES 20
> > - for ( i = 0; i < MAX_PRINTED_BYTES; i++ )
> > - {
> > + for (i = 0; i < MAX_PRINTED_BYTES; i++) {
> > FPU_get_user(byte1, (u_char __user *) address);
> > - if ( (byte1 & 0xf8) == 0xd8 )
> > - {
> > + if ((byte1 & 0xf8) == 0xd8) {
> > printk(" %02x", byte1);
> > break;
> > }
> > printk(" [%02x]", byte1);
> > address++;
> > }
> > - if ( i == MAX_PRINTED_BYTES )
> > + if (i == MAX_PRINTED_BYTES)
> > printk(" [more..]\n");
> > - else
> > - {
> > + else {
> > FPU_get_user(FPU_modrm, 1 + (u_char __user *) address);
> > -
> > +
> > if (FPU_modrm >= 0300)
> > - printk(" %02x (%02x+%d)\n", FPU_modrm, FPU_modrm & 0xf8, FPU_modrm & 7);
> > + printk(" %02x (%02x+%d)\n", FPU_modrm, FPU_modrm & 0xf8,
> > + FPU_modrm & 7);
> > else
> > printk(" /%d, mod=%d rm=%d\n",
> > (FPU_modrm >> 3) & 7, (FPU_modrm >> 6) & 3, FPU_modrm & 7);
> > }
> > }
> > else
> > - {
> > printk("%04x\n", FPU_CS);
> > - }
> >
> > partial_status = status_word();
> >
> > #ifdef DEBUGGING
> > -if ( partial_status & SW_Backward ) printk("SW: backward compatibility\n");
> > -if ( partial_status & SW_C3 ) printk("SW: condition bit 3\n");
> > -if ( partial_status & SW_C2 ) printk("SW: condition bit 2\n");
> > -if ( partial_status & SW_C1 ) printk("SW: condition bit 1\n");
> > -if ( partial_status & SW_C0 ) printk("SW: condition bit 0\n");
> > -if ( partial_status & SW_Summary ) printk("SW: exception summary\n");
> > -if ( partial_status & SW_Stack_Fault ) printk("SW: stack fault\n");
> > -if ( partial_status & SW_Precision ) printk("SW: loss of precision\n");
> > -if ( partial_status & SW_Underflow ) printk("SW: underflow\n");
> > -if ( partial_status & SW_Overflow ) printk("SW: overflow\n");
> > -if ( partial_status & SW_Zero_Div ) printk("SW: divide by zero\n");
> > -if ( partial_status & SW_Denorm_Op ) printk("SW: denormalized operand\n");
> > -if ( partial_status & SW_Invalid ) printk("SW: invalid operation\n");
> > +if (partial_status & SW_Backward)
> > + printk("SW: backward compatibility\n");
> > +if (partial_status & SW_C3)
> > + printk("SW: condition bit 3\n");
> > +if (partial_status & SW_C2)
> > + printk("SW: condition bit 2\n");
> > +if (partial_status & SW_C1)
> > + printk("SW: condition bit 1\n");
> > +if (partial_status & SW_C0)
> > + printk("SW: condition bit 0\n");
> > +if (partial_status & SW_Summary)
> > + printk("SW: exception summary\n");
> > +if (partial_status & SW_Stack_Fault)
> > + printk("SW: stack fault\n");
> > +if (partial_status & SW_Precision)
> > + printk("SW: loss of precision\n");
> > +if (partial_status & SW_Underflow)
> > + printk("SW: underflow\n");
> > +if (partial_status & SW_Overflow)
> > + printk("SW: overflow\n");
> > +if (partial_status & SW_Zero_Div)
> > + printk("SW: divide by zero\n");
> > +if (partial_status & SW_Denorm_Op)
> > + printk("SW: denormalized operand\n");
> > +if (partial_status & SW_Invalid)
> > + printk("SW: invalid operation\n");
>
> While you're at it, please think about adding proper KERN_ facility to these printk() calls.
Yes, basically all the warnings are about missing KERN_.
Do you want me to add KERN_ERR to all the above printk?
> > #endif /* DEBUGGING */
> >
> > printk(" SW: b=%d st=%ld es=%d sf=%d cc=%d%d%d%d ef=%d%d%d%d%d%d\n",
>
> Ditto.
>
> > @@ -155,7 +161,7 @@ if ( partial_status & SW_Invalid ) printk("SW: invalid operation\n");
> > partial_status & SW_Precision?1:0, partial_status & SW_Underflow?1:0,
> > partial_status & SW_Overflow?1:0, partial_status & SW_Zero_Div?1:0,
> > partial_status & SW_Denorm_Op?1:0, partial_status & SW_Invalid?1:0);
> > -
> > +
> > printk(" CW: ic=%d rc=%ld%ld pc=%ld%ld iem=%d ef=%d%d%d%d%d%d\n",
>
> Ditto.
>
[...]
> > #ifdef __DEBUG__
> > - math_abort(FPU_info,SIGFPE);
> > + math_abort(FPU_info , SIGFPE);
>
> Why do you need this extra space before the comma?
My mistake.
> This file seems to be in need of formatting lines to use tabs instead of spaces.
>
Ciao,
--
Paolo
http://paolo.ciarrocchi.googlepages.com/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists