[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH][RFC]xenperf hypercall pretty print



> On Fri, 2006-08-18 at 18:48 +0900, Ken Hironaka wrote:
> > +const char* hypercall_name_table[64]=
> > +{
> > +    "set_trap_table",        //0
> > +    "mmu_update",            //1
> > +    "set_gdt",               //2
>
> ...
>
> Rather than have all those numbers in the comments, it would be better
> to use the C99 array initialization syntax:
>
> const char* hypercall_name_table[64] = {
>       [__HYPERVISOR_set_trap_table] = "set_trap_table",
>       [__HYPERVISOR_mmu_update] = "mmu_update",
> ...
> };

Agreed!

Also, just thought I'd be pedantic and mention we generally seek to avoid C++ 
style comments.  We seem to have standardised on /* */ comments.

Sorry, Ken, I know it's geeky but I saw those slashes and couldn't resist!  
Maybe it'll be useful for future patches.

Cheers,
Mark

> > @@ -117,10 +185,17 @@ int main(int argc, char *argv[])
> >              sum += val[j];
> >          printf ("T=%10u ", (unsigned int)sum);
> >
> > -        if ( full || (pcd[i].nr_vals <= 4) )
> > +        if ( full || (pcd[i].nr_vals <= 4) ){
> > +                       if( strcmp(pcd[i].name, "hypercalls") == 0 ){
> > +                               for( j = 0; j < pcd[i].nr_vals; j++ )
> > +                                       printf("%s\t",
> > hypercall_name_table[j]);
> > +                               printf("\n");
> > +                       }
> >              for ( j = 0; j < pcd[i].nr_vals; j++ )
> >                  printf(" %10u", (unsigned int)val[j]);
> >
> > +               }
> > +
> >          printf("\n");
> >                 val += pcd[i].nr_vals;
> >      }
>
> Looks like the whitespace got out of hand here. Please double-check.
>
> Also, it looks like this file has had some tabs creep into it. You
> should probably send a tab-removal patch first, then this patch on top
> of that.

-- 
Dave: Just a question. What use is a unicyle with no seat?  And no pedals!
Mark: To answer a question with a question: What use is a skateboard?
Dave: Skateboards have wheels.
Mark: My wheel has a wheel!

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.