Re: [Xen-devel] [PATCH v1 2/8]: PVH mmu changes

[Konrad Rzeszutek Wilk <konrad@xxxxxxxxxx>]

> > > > > +     struct page **pi_paga;          /* pfn info page
> > > > > array */
> > > > 
> > > > can we just call this "pages"? paga is pretty meaningless.
> > > 
> > > page array! i can rename page_array or page_a.
> > 
> > What's wrong with pages? It's short (some of the lines using this
> > stuff are necessarily pretty long) and obvious.
> 
> grep'ing pages would give thousands results. I can prefix with something
> and use pages. 

Please don't prefix it. 'pages' is good.
> 
> 
> > > > > +     int           pi_num_pgs;
> > > > > +     int           pi_next_todo;
> > > > 
> > > > I don't think we need the pi_ prefix for any of these.
> > > 
> > > The prefix for fields in struct make it easy to find via cscope or
> > > grep, otherwise, it's a nightmare to find common field names like
> > > pages when reading code. I really get frustrated. I prefer prefixing
> > > all field names.
> > 
> > It's not common practice in Linux to do so but fair enough.

Please remove the 'pi_' field. Everytime I see it I think of bathroom
and then 3.1415... I've no idea what it actually stands for and I am
not sure if there is a need to know what it stands for? If the 'pi_'
is very important - it should be part of the structure's name.
And then probably unrolled.

If you are searching for a field in a structure - why? Why not
search for the structure itself? Making the structure name
unique should be enough right to find in cscope/ctags?

Both ctags and cscope are good at helping you (once you have the
structure or code name) at finding the definitions of the fields if
you need to.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel