[coreinfo] [PATCH] fix cpu flags overwrite

[Stefan Reinauer]

See patch

[Ulf Jordan]

On Thu, 18 Sep 2008, Stefan Reinauer wrote:
> See patch
>
> After dumping the ecx flags, decode_flags starts in the same line for 
> the edx flags if less than 16 flags were decoded. Some flags could not 
> be read because they're overwritten. This patch fixes the problem by 
> forcing a new line for each decode.
>
> Signed-off-by: Stefan Reinauer

With the two comments below addressed, this patch is

Acked-by: Ulf Jordan
> Index: coreinfo/cpuinfo_module.c
>
> --- coreinfo/cpuinfo_module.c   (revision 3569)
> +++ coreinfo/cpuinfo_module.c   (working copy)
> @@ -97,6 +97,7 @@
>  {
>         int i;
>         int lrow = *row;
>   int output_available=0;
>
>   wmove(win, lrow, 2);
>
> @@ -104,8 +105,10 @@
>                 if (flags[i] == NULL)
>                         continue;
>           if (reg & (1 << i))
>           if (reg & (1 << i)) {
>                   wprintw(win, "%s ", flags[i]);
>                   output_available=1;
>           }
>
>           if (i && (i % 16) == 0) {
>                   lrow++;
>
> @@ -113,6 +116,8 @@
>                 }
>         }
>   lrow += output_available;
>   *row = lrow;
>
> }
>
> @@ -131,7 +136,7 @@
>     switch (vendor) {
>     case VENDOR_AMD:
>           wmove(win, lrow++, 1);
>           wmove(win, lrow, 1);
>           wprintw(win, "AMD Extended Flags: ");

This hunk will make the decoded flags overwrite the heading, please 
remove.
>             decode_flags(win, ecx, amd_cap_generic_ecx_flags, &lrow);
>             docpuid(0x80000001, &eax, &ebx, &ecx, &edx);
>
> @@ -139,7 +144,7 @@
>                 decode_flags(win, ecx, amd_cap_extended_ecx_flags, &lrow);
>                 break;
>         case VENDOR_INTEL:
>           wmove(win, lrow++, 1);
>           wmove(win, lrow, 1);
>           wprintw(win, "Intel Extended Flags: ");

Ditto.
>             decode_flags(win, ecx, intel_cap_generic_ecx_flags, &lrow);
>             docpuid(0x80000001, &eax, &ebx, &ecx, &edx);

/ulf

[Stefan Reinauer]

Hi Ulf,

Ulf Jordan wrote:
> > Index: coreinfo/cpuinfo_module.c
> >
> > --- coreinfo/cpuinfo_module.c   (revision 3569)
> > +++ coreinfo/cpuinfo_module.c   (working copy)
> > @@ -97,6 +97,7 @@
> >  {
> >         int i;
> >         int lrow = *row;
> >   int output_available=0;
> >
> >   wmove(win, lrow, 2);
> >
> > @@ -104,8 +105,10 @@
> >                 if (flags[i] == NULL)
> >                         continue;
> >           if (reg & (1 << i))
> >           if (reg & (1 << i)) {
> >                   wprintw(win, "%s ", flags[i]);
> >                   output_available=1;
> >           }
> >
> >           if (i && (i % 16) == 0) {
> >                   lrow++;
> >
> > @@ -113,6 +116,8 @@
> >                 }
> >         }
> >   lrow += output_available;
> >   *row = lrow;
> >
> > }
> >
> > @@ -131,7 +136,7 @@
> >     switch (vendor) {
> >     case VENDOR_AMD:
> >           wmove(win, lrow++, 1);
> >           wmove(win, lrow, 1);
> >           wprintw(win, "AMD Extended Flags: ");
>
> This hunk will make the decoded flags overwrite the heading, please
> remove.

In which case would that happen? The   lrow += output_available; part
above prevents this on my system. Does it not work as intended on your
machine?

Stefan

[Ulf Jordan]

Hello Stefan,

On Fri, 26 Sep 2008, Stefan Reinauer wrote:
> Hi Ulf,

Ulf Jordan wrote:
> > Index: coreinfo/cpuinfo_module.c
> >
> > --- coreinfo/cpuinfo_module.c   (revision 3569)
> > +++ coreinfo/cpuinfo_module.c   (working copy)
> > @@ -97,6 +97,7 @@
> >  {
> >         int i;
> >         int lrow = *row;
> >   int output_available=0;
> >
> >   wmove(win, lrow, 2);
> >
> > @@ -104,8 +105,10 @@
> >                 if (flags[i] == NULL)
> >                         continue;
> >           if (reg & (1 << i))
> >           if (reg & (1 << i)) {
> >                   wprintw(win, "%s ", flags[i]);
> >                   output_available=1;
> >           }
> >
> >           if (i && (i % 16) == 0) {
> >                   lrow++;
> >
> > @@ -113,6 +116,8 @@
> >                 }
> >         }
> >   lrow += output_available;
> >   *row = lrow;
> >
> > }
> >
> > @@ -131,7 +136,7 @@
> >     switch (vendor) {
> >     case VENDOR_AMD:
> >           wmove(win, lrow++, 1);
> >           wmove(win, lrow, 1);
> >           wprintw(win, "AMD Extended Flags: ");
>
> This hunk will make the decoded flags overwrite the heading, please
> remove.

In which case would that happen? The   lrow += output_available; part
above prevents this on my system. Does it not work as intended on your
machine?

lrow += output_available; makes certain that there is space *after* 
already written flags, but this is another issue.

By removing the ++, the extended flags are written onto the "Intel 
Extended Flags:/AMD Extended Flags:" heading, starting in position three. 
I've tested in QEMU, and the result is as follows.

Without patch:
  Features:
   fpu de pse tsc msr pae mce cx8 apic sep pge cmov pat
   mmx fxsr sse sse2
  Intel Extended Flags:
   sse3

With patch:
  Features:
   fpu de pse tsc msr pae mce cx8 apic sep pge cmov pat
   mmx fxsr sse sse2

Isse3 Extended Flags:

With patch, excluding last two hunks:
  Features:
   fpu de pse tsc msr pae mce cx8 apic sep pge cmov pat
   mmx fxsr sse sse2

Intel Extended Flags:
   sse3

The reason for overwriting when ++ is removed is that wmove positions the 
cursor at current lrow and wprintw prints the heading. Directly 
afterwards, decode_flags starts writing the decode flags at lrow 
(unchanged), but at x = 2.

This is the reason I suggested the patch to be applied, except for the 
last two hunks.

Regards,

Ulf