lkml.org 
[lkml]   [2011]   [Mar]   [2]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 3/6] Staging: hv: Cleanup hyperv_device variable names
On Wed, Mar 02, 2011 at 01:42:37AM +0000, KY Srinivasan wrote:
>
> > -----Original Message-----
> > From: Greg KH [mailto:greg@kroah.com]
> > Sent: Monday, February 28, 2011 9:44 PM
> > To: KY Srinivasan
> > Cc: gregkh@suse.de; linux-kernel@vger.kernel.org;
> > devel@linuxdriverproject.org; virtualization@lists.osdl.org; Haiyang Zhang; Hank
> > Janssen
> > Subject: Re: [PATCH 3/6] Staging: hv: Cleanup hyperv_device variable names
> >
> > On Fri, Feb 25, 2011 at 06:06:32PM -0800, K. Y. Srinivasan wrote:
> > > Cleanup the names of variables that refer to the
> > > hyperv_device abstraction.
> >
> > Clean them up to be what? Shorter? Nice? Full of rounded edges so
> > that when we bump into them in the dark they don't poke us and cause us
> > to shreak in pain?
> >
> > >
> > >
> > > Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> > > Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> >
> > Sweet, you cloned yourself, I thought only Alan Cox had achieved that
> > goal...
> >
> > > Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
> > > Signed-off-by: Hank Janssen <hjanssen@microsoft.com>
> > >
> > > ---
> > > drivers/staging/hv/blkvsc_drv.c | 12 ++--
> > > drivers/staging/hv/netvsc.c | 4 +-
> > > drivers/staging/hv/netvsc_drv.c | 36 ++++----
> > > drivers/staging/hv/storvsc_drv.c | 44 +++++-----
> > > drivers/staging/hv/vmbus_drv.c | 164 +++++++++++++++++++-----------------
> > --
> > > 5 files changed, 130 insertions(+), 130 deletions(-)
> > >
> > > diff --git a/drivers/staging/hv/blkvsc_drv.c b/drivers/staging/hv/blkvsc_drv.c
> > > index 58ab0e8..305a665 100644
> > > --- a/drivers/staging/hv/blkvsc_drv.c
> > > +++ b/drivers/staging/hv/blkvsc_drv.c
> > > @@ -95,7 +95,7 @@ struct blkvsc_request {
> > > /* Per device structure */
> > > struct block_device_context {
> > > /* point back to our device context */
> > > - struct hyperv_device *device_ctx;
> > > + struct hyperv_device *device_obj;
> >
> > Hey, I was right, it does have more rounded edges, nicely done.
> >
> >
> > > -static int netvsc_device_add(struct hyperv_device *device,
> > > - void *additional_info);
> > > +static int
> > > +netvsc_device_add(struct hyperv_device *device, void *additional_info);
> >
> > Again with the function return value hiding. Please don't.
> >
> > > --- a/drivers/staging/hv/storvsc_drv.c
> > > +++ b/drivers/staging/hv/storvsc_drv.c
> > > @@ -43,7 +43,7 @@ struct host_device_context {
> > > /* must be 1st field
> > > * FIXME this is a bug */
> > > /* point back to our device context */
> > > - struct hyperv_device *device_ctx;
> > > + struct hyperv_device *device_obj;
> >
> > I really don't understand this change at all. "obj" is just as vapid
> > and clueless as "ctx" is, and it seems very gratuitous to change this.
> > And I should know, I have made a lot of gratuitous renames in my time in
> > the kernel...
>
> Greg, there are not that many options here. As I recall there was universal
> objection to the use of *context/*ctx to refer to device or driver objects.
> The name I chose is fairly descriptive of what it represents. If there is consensus
> on a better name, I will use it.

Do it like other subsystems, call it a device with the prefix for the
type. So for this one, it would be:
struct hyperv_device *hyperv_device;
or
struct hyperv_device *hyperv_dev;

> > Come on, global search-and-replace needs to be done in a sane manner,
> > other wise you can just send me a vi macro to run on the code, it would
> > be the same thing in the end (hint, don't do that, only one person has
> > ever gotten away with doing that in the history of the kernel, in an act
> > never to be ever repeated again.)
>
> Greg, apart from your objection to the name I picked to refer to variable
> referring to struct hyperv_device; what else is the problem here.

Changing the comment that should be removed instead.

thanks,

greg k-h


\
 
 \ /
  Last update: 2011-03-02 06:49    [from the cache]
©2003-2011 Jasper Spaans