From mboxrd@z Thu Jan 1 00:00:00 1970 From: Konrad Rzeszutek Wilk Subject: Re: [PATCH v1 2/8]: PVH mmu changes Date: Thu, 4 Oct 2012 14:42:55 -0400 Message-ID: <20121004184254.GA28198@phenom.dumpdata.com> References: <20120921121556.1a0ea8af@mantra.us.oracle.com> <1349278963.650.164.camel@zakaz.uk.xensource.com> <20121003152925.5af3a658@mantra.us.oracle.com> <1349339279.650.214.camel@zakaz.uk.xensource.com> <20121004112734.4f003584@mantra.us.oracle.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Content-Disposition: inline In-Reply-To: <20121004112734.4f003584@mantra.us.oracle.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Mukesh Rathor Cc: "Xen-devel@lists.xensource.com" , Ian Campbell , Konrad Rzeszutek Wilk , Stefano Stabellini List-Id: xen-devel@lists.xenproject.org > > > > > + 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.