subject:"Re\: \[PATCH 3\/6\] Staging\: hv\: Cleanup hyperv

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

2011-03-01 Thread KY Srinivasan



 -Original Message-
 From: Greg KH [mailto:g...@kroah.com]
 Sent: Monday, February 28, 2011 9:44 PM
 To: KY Srinivasan
 Cc: gre...@suse.de; linux-ker...@vger.kernel.org;
 de...@linuxdriverproject.org; virtualizat...@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 k...@microsoft.com
  Signed-off-by: K. Y. Srinivasan k...@microsoft.com
 
 Sweet, you cloned yourself, I thought only Alan Cox had achieved that
 goal...
 
  Signed-off-by: Haiyang Zhang haiya...@microsoft.com
  Signed-off-by: Hank Janssen hjans...@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. 

 
   static int vmbus_uevent(struct device *device, struct kobj_uevent_env *env)
   {
  -   struct hyperv_device *device_ctx = device_to_hyperv_device(device);
  +   struct hyperv_device *device_obj = device_to_hyperv_device(device);
  int ret;
 
  DPRINT_INFO(VMBUS_DRV, generating uevent -
 VMBUS_DEVICE_CLASS_GUID={
  %02x%02x%02x%02x-%02x%02x-%02x%02x-
  %02x%02x%02x%02x%02x%02x%02x%02x},
  -   device_ctx-class_id.data[3], device_ctx-class_id.data[2],
  -   device_ctx-class_id.data[1], device_ctx-class_id.data[0],
  -   device_ctx-class_id.data[5], device_ctx-class_id.data[4],
  -   device_ctx-class_id.data[7], device_ctx-class_id.data[6],
  -   device_ctx-class_id.data[8], device_ctx-class_id.data[9],
  -   device_ctx-class_id.data[10],
  -   device_ctx-class_id.data[11],
  -   device_ctx-class_id.data[12],
  -   device_ctx-class_id.data[13],
  -   device_ctx-class_id.data[14],
  -   device_ctx-class_id.data[15]);
  +   device_obj-class_id.data[3], device_obj-class_id.data[2],
  +   device_obj-class_id.data[1], device_obj-class_id.data[0],
  +   device_obj-class_id.data[5], device_obj-class_id.data[4],
  +   device_obj-class_id.data[7], device_obj-class_id.data[6],
  +   device_obj-class_id.data[8], device_obj-class_id.data[9],
  +   device_obj-class_id.data[10],
  +   device_obj-class_id.data[11],
  +   device_obj-class_id.data[12],
  +   device_obj-class_id.data[13],
  +   device_obj-class_id.data[14],
  +   device_obj-class_id.data[15]);
 
 After seeing this stuff so many times I'm waiting for a helper function
 to be added for it in this subsystem.  I'm sure you really don't want to
 edit GUID printk-like functions ever again, right?

I will have a separate patch for this.

 
   static void vmbus_device_release(struct device *device)
   {
  -   struct

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

2011-03-01 Thread Greg KH

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

  -Original Message-
  From: Greg KH [mailto:g...@kroah.com]
  Sent: Monday, February 28, 2011 9:44 PM
  To: KY Srinivasan
  Cc: gre...@suse.de; linux-ker...@vger.kernel.org;
  de...@linuxdriverproject.org; virtualizat...@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 k...@microsoft.com
   Signed-off-by: K. Y. Srinivasan k...@microsoft.com

  Sweet, you cloned yourself, I thought only Alan Cox had achieved that
  goal...

   Signed-off-by: Haiyang Zhang haiya...@microsoft.com
   Signed-off-by: Hank Janssen hjans...@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
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/virtualization

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

2011-02-28 Thread Greg KH

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 k...@microsoft.com
 Signed-off-by: K. Y. Srinivasan k...@microsoft.com

Sweet, you cloned yourself, I thought only Alan Cox had achieved that
goal...

 Signed-off-by: Haiyang Zhang haiya...@microsoft.com
 Signed-off-by: Hank Janssen hjans...@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...

  static int vmbus_uevent(struct device *device, struct kobj_uevent_env *env)
  {
 - struct hyperv_device *device_ctx = device_to_hyperv_device(device);
 + struct hyperv_device *device_obj = device_to_hyperv_device(device);
   int ret;
  
   DPRINT_INFO(VMBUS_DRV, generating uevent - VMBUS_DEVICE_CLASS_GUID={
   %02x%02x%02x%02x-%02x%02x-%02x%02x-
   %02x%02x%02x%02x%02x%02x%02x%02x},
 - device_ctx-class_id.data[3], device_ctx-class_id.data[2],
 - device_ctx-class_id.data[1], device_ctx-class_id.data[0],
 - device_ctx-class_id.data[5], device_ctx-class_id.data[4],
 - device_ctx-class_id.data[7], device_ctx-class_id.data[6],
 - device_ctx-class_id.data[8], device_ctx-class_id.data[9],
 - device_ctx-class_id.data[10],
 - device_ctx-class_id.data[11],
 - device_ctx-class_id.data[12],
 - device_ctx-class_id.data[13],
 - device_ctx-class_id.data[14],
 - device_ctx-class_id.data[15]);
 + device_obj-class_id.data[3], device_obj-class_id.data[2],
 + device_obj-class_id.data[1], device_obj-class_id.data[0],
 + device_obj-class_id.data[5], device_obj-class_id.data[4],
 + device_obj-class_id.data[7], device_obj-class_id.data[6],
 + device_obj-class_id.data[8], device_obj-class_id.data[9],
 + device_obj-class_id.data[10],
 + device_obj-class_id.data[11],
 + device_obj-class_id.data[12],
 + device_obj-class_id.data[13],
 + device_obj-class_id.data[14],
 + device_obj-class_id.data[15]);

After seeing this stuff so many times I'm waiting for a helper function
to be added for it in this subsystem.  I'm sure you really don't want to
edit GUID printk-like functions ever again, right?

  static void vmbus_device_release(struct device *device)
  {
 - struct hyperv_device *device_ctx = device_to_hyperv_device(device);
 + struct hyperv_device *device_obj = device_to_hyperv_device(device);
  
 - kfree(device_ctx);
 + kfree(device_obj);
  
 - /* !!DO NOT REFERENCE device_ctx anymore at this point!! */
 + /* !!DO NOT REFERENCE device_obj anymore at this point!! */
  }

I think by virtue of the kfree() right above this comment, it should be
deleted.  Especially as it is at the end of the function, making it
pretty much impossible to make any sense here...

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

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

2011-02-26 Thread KY Srinivasan



 -Original Message-
 From: Dan Carpenter [mailto:erro...@gmail.com]
 Sent: Friday, February 25, 2011 10:33 PM
 To: KY Srinivasan
 Cc: gre...@suse.de; linux-ker...@vger.kernel.org;
 de...@linuxdriverproject.org; virtualizat...@lists.osdl.org
 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.
 
 
 I have a script I run on rename patches.  For this patch the
 command would be:
 
 rename_rev.pl -nc -e 's/device_ctx/device_obj/g' -e 's/\bdev_ctx/dev/g'
 
 It show one variable was missed in storvsc_probe().
 
  memset(host_device_ctx, 0, sizeof(struct host_device_context));
 
  host_device_ctx-port = host-host_no;
  -   host_device_ctx-device_ctx = device_obj;
  +   host_device_ctx-device_obj = device_obj;
 
  host_device_ctx-request_pool =
  kmem_cache_create(dev_name(device_obj-
 device),
  @@ -271,7 +271,7 @@ static int storvsc_probe(struct device *device)
  return -1;
  }
 
  -   /* host_device_ctx-port = device_info.PortNumber; */
  +   /* host_hyperv_dev-port = device_info.PortNumber; */
  host_device_ctx-path = device_info.path_id;
  host_device_ctx-target = device_info.target_id;
 
 
 All the other host_device_ctx variables were renamed to host_device_obj.

host_device_ctx is a scsi abstraction built on top of the hyperv_device
abstraction. My goal was not to change the names of higher level scsi
abstractions.

Regards,

K. Y
 
 regards,
 dan carpenter

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/virtualization

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

2011-02-25 Thread Dan Carpenter

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.
 

I have a script I run on rename patches.  For this patch the
command would be:

rename_rev.pl -nc -e 's/device_ctx/device_obj/g' -e 's/\bdev_ctx/dev/g'

It show one variable was missed in storvsc_probe().  

   memset(host_device_ctx, 0, sizeof(struct host_device_context));
  
   host_device_ctx-port = host-host_no;
 - host_device_ctx-device_ctx = device_obj;
 + host_device_ctx-device_obj = device_obj;
  
   host_device_ctx-request_pool =
   kmem_cache_create(dev_name(device_obj-device),
 @@ -271,7 +271,7 @@ static int storvsc_probe(struct device *device)
   return -1;
   }
  
 - /* host_device_ctx-port = device_info.PortNumber; */
 + /* host_hyperv_dev-port = device_info.PortNumber; */
   host_device_ctx-path = device_info.path_id;
   host_device_ctx-target = device_info.target_id;
  

All the other host_device_ctx variables were renamed to host_device_obj.

regards,
dan carpenter
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/virtualization

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

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

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

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

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

5 matches

Site Navigation

Mail list logo

Footer information