All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] Staging: greybus: usb: Fixed a coding style error
@ 2019-04-01 14:22 Will Cunningham
  2019-04-01 14:25 ` Alex Elder
  2019-04-02  6:07 ` Dan Carpenter
  0 siblings, 2 replies; 3+ messages in thread
From: Will Cunningham @ 2019-04-01 14:22 UTC (permalink / raw)
  To: johan; +Cc: elder, gregkh, devel, linux-kernel, joe, elder

Line was >80 characters.

Signed-off-by: Will Cunningham <wjcunningham7@gmail.com>
---
Changes in v2:
 - Created a tmp variable to shorten line length.
---
 drivers/staging/greybus/usb.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/greybus/usb.c b/drivers/staging/greybus/usb.c
index 1c246c73a085..c09793b48850 100644
--- a/drivers/staging/greybus/usb.c
+++ b/drivers/staging/greybus/usb.c
@@ -163,14 +163,14 @@ static int gb_usb_probe(struct gbphy_device *gbphy_dev,
 	struct gb_usb_device *gb_usb_dev;
 	struct usb_hcd *hcd;
 	int retval;
+	u16 tmp;
 
 	hcd = usb_create_hcd(&usb_gb_hc_driver, dev, dev_name(dev));
 	if (!hcd)
 		return -ENOMEM;
 
-	connection = gb_connection_create(gbphy_dev->bundle,
-					  le16_to_cpu(gbphy_dev->cport_desc->id),
-					  NULL);
+	tmp = le16_to_cpu(gbphy_dev->cport_desc->id);
+	connection = gb_connection_create(gbphy_dev->bundle, tmp, NULL);
 	if (IS_ERR(connection)) {
 		retval = PTR_ERR(connection);
 		goto exit_usb_put;
-- 
2.19.2


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH v2] Staging: greybus: usb: Fixed a coding style error
  2019-04-01 14:22 [PATCH v2] Staging: greybus: usb: Fixed a coding style error Will Cunningham
@ 2019-04-01 14:25 ` Alex Elder
  2019-04-02  6:07 ` Dan Carpenter
  1 sibling, 0 replies; 3+ messages in thread
From: Alex Elder @ 2019-04-01 14:25 UTC (permalink / raw)
  To: Will Cunningham, johan; +Cc: elder, gregkh, devel, linux-kernel, joe

On 4/1/19 9:22 AM, Will Cunningham wrote:
> Line was >80 characters.

This looks fine, but "tmp" is not a meaningful name.
That argument to gb_connection_create() is a cport id,
so "cport_id" would be a much better name for the variable.

It seems picky, but details like this make the code much
more understandable.

					-Alex


> Signed-off-by: Will Cunningham <wjcunningham7@gmail.com>
> ---
> Changes in v2:
>  - Created a tmp variable to shorten line length.
> ---
>  drivers/staging/greybus/usb.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/staging/greybus/usb.c b/drivers/staging/greybus/usb.c
> index 1c246c73a085..c09793b48850 100644
> --- a/drivers/staging/greybus/usb.c
> +++ b/drivers/staging/greybus/usb.c
> @@ -163,14 +163,14 @@ static int gb_usb_probe(struct gbphy_device *gbphy_dev,
>  	struct gb_usb_device *gb_usb_dev;
>  	struct usb_hcd *hcd;
>  	int retval;
> +	u16 tmp;
>  
>  	hcd = usb_create_hcd(&usb_gb_hc_driver, dev, dev_name(dev));
>  	if (!hcd)
>  		return -ENOMEM;
>  
> -	connection = gb_connection_create(gbphy_dev->bundle,
> -					  le16_to_cpu(gbphy_dev->cport_desc->id),
> -					  NULL);
> +	tmp = le16_to_cpu(gbphy_dev->cport_desc->id);
> +	connection = gb_connection_create(gbphy_dev->bundle, tmp, NULL);
>  	if (IS_ERR(connection)) {
>  		retval = PTR_ERR(connection);
>  		goto exit_usb_put;
> 


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v2] Staging: greybus: usb: Fixed a coding style error
  2019-04-01 14:22 [PATCH v2] Staging: greybus: usb: Fixed a coding style error Will Cunningham
  2019-04-01 14:25 ` Alex Elder
@ 2019-04-02  6:07 ` Dan Carpenter
  1 sibling, 0 replies; 3+ messages in thread
From: Dan Carpenter @ 2019-04-02  6:07 UTC (permalink / raw)
  To: Will Cunningham; +Cc: johan, devel, elder, gregkh, linux-kernel, elder, joe

On Mon, Apr 01, 2019 at 10:22:08AM -0400, Will Cunningham wrote:
> Line was >80 characters.
> 
> Signed-off-by: Will Cunningham <wjcunningham7@gmail.com>
> ---
> Changes in v2:
>  - Created a tmp variable to shorten line length.
> ---
>  drivers/staging/greybus/usb.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/staging/greybus/usb.c b/drivers/staging/greybus/usb.c
> index 1c246c73a085..c09793b48850 100644
> --- a/drivers/staging/greybus/usb.c
> +++ b/drivers/staging/greybus/usb.c
> @@ -163,14 +163,14 @@ static int gb_usb_probe(struct gbphy_device *gbphy_dev,
>  	struct gb_usb_device *gb_usb_dev;
>  	struct usb_hcd *hcd;
>  	int retval;
> +	u16 tmp;
>  
>  	hcd = usb_create_hcd(&usb_gb_hc_driver, dev, dev_name(dev));
>  	if (!hcd)
>  		return -ENOMEM;
>  
> -	connection = gb_connection_create(gbphy_dev->bundle,
> -					  le16_to_cpu(gbphy_dev->cport_desc->id),
> -					  NULL);
> +	tmp = le16_to_cpu(gbphy_dev->cport_desc->id);
> +	connection = gb_connection_create(gbphy_dev->bundle, tmp, NULL);


"tmp" is the wrong name...  :/

Like Joe said, it's doesn't help readability to introduce one time use
temporary variables just to make the line lengths shorter.  This line is
81 characters.  It's fine.  Just leave the original as is.

regards,
dan carpenter


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2019-04-02  6:07 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-01 14:22 [PATCH v2] Staging: greybus: usb: Fixed a coding style error Will Cunningham
2019-04-01 14:25 ` Alex Elder
2019-04-02  6:07 ` Dan Carpenter

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.