lkml.org 
[lkml]   [2011]   [Mar]   [2]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    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    [W:0.023 / U:124.720 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site