On Wed, Mar 02, 2011 at 01:42:37AM +0000, KY Srinivasan wrote:
>
>
> > -----Original Message-----
> > From: Greg KH [mailto:[email protected]]
> > Sent: Monday, February 28, 2011 9:44 PM
> > To: KY Srinivasan
> > Cc: [email protected]; [email protected];
> > [email protected]; [email protected]; 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 <[email protected]>
> > > Signed-off-by: K. Y. Srinivasan <[email protected]>
> >
> > Sweet, you cloned yourself, I thought only Alan Cox had achieved that
> > goal...
> >
> > > Signed-off-by: Haiyang Zhang <[email protected]>
> > > Signed-off-by: Hank Janssen <[email protected]>
> > >
> > > ---
> > > 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
_______________________________________________
devel mailing list
[email protected]
http://driverdev.linuxdriverproject.org/mailman/listinfo/devel
