Re: [PATCH 3/6] Staging: hv: Cleanup hyperv_device variable names

On Wed, Mar 02, 2011 at 01:42:37AM +0000, KY Srinivasan wrote:

quoted

-----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:

quoted

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?

quoted

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...

quoted

Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
Signed-off-by: Hank Janssen <redacted>

---
 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 +++++++++++++++++++-----------------

--

quoted

 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.

quoted

-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.

quoted

--- 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;

quoted

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