DriverDev-Devel Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH] Fixed parentheses malpractice in apex_driver.c
@ 2019-09-06 18:38 volery
  2019-09-06 20:55 ` Joe Perches
  2019-09-07 14:38 ` Dan Carpenter
  0 siblings, 2 replies; 5+ messages in thread
From: volery @ 2019-09-06 18:38 UTC (permalink / raw)
  To: rspringer, toddpoynor, benchan, gregkh, devel, linux-kernel

There were some parentheses at the end of lines, which I took care of.
This is my first patch.

Signed-off-by: Sandro Volery <sandro@volery.com>
---
 drivers/staging/gasket/apex_driver.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/gasket/apex_driver.c b/drivers/staging/gasket/apex_driver.c
index 464648ee2036..78ebd590f877 100644
--- a/drivers/staging/gasket/apex_driver.c
+++ b/drivers/staging/gasket/apex_driver.c
@@ -527,17 +527,20 @@ static ssize_t sysfs_show(struct device *device, struct device_attribute *attr,
 	switch (type) {
 	case ATTR_KERNEL_HIB_PAGE_TABLE_SIZE:
 		ret = scnprintf(buf, PAGE_SIZE, "%u\n",
-				gasket_page_table_num_entries(
+				gasket_page_table_num_entries
+				(
 					gasket_dev->page_table[0]));
 		break;
 	case ATTR_KERNEL_HIB_SIMPLE_PAGE_TABLE_SIZE:
 		ret = scnprintf(buf, PAGE_SIZE, "%u\n",
-				gasket_page_table_num_simple_entries(
+				gasket_page_table_num_simple_entries
+				(
 					gasket_dev->page_table[0]));
 		break;
 	case ATTR_KERNEL_HIB_NUM_ACTIVE_PAGES:
 		ret = scnprintf(buf, PAGE_SIZE, "%u\n",
-				gasket_page_table_num_active_pages(
+				gasket_page_table_num_active_pages
+				(
 					gasket_dev->page_table[0]));
 		break;
 	default:
-- 
2.23.0

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

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

* Re: [PATCH] Fixed parentheses malpractice in apex_driver.c
  2019-09-06 18:38 [PATCH] Fixed parentheses malpractice in apex_driver.c volery
@ 2019-09-06 20:55 ` Joe Perches
  2019-09-07 14:38 ` Dan Carpenter
  1 sibling, 0 replies; 5+ messages in thread
From: Joe Perches @ 2019-09-06 20:55 UTC (permalink / raw)
  To: volery, rspringer, toddpoynor, benchan, gregkh, devel, linux-kernel

On Fri, 2019-09-06 at 20:38 +0200, volery wrote:
> There were some parentheses at the end of lines, which I took care of.

Not every instance of this checkpatch warning should be changed.

This specific instance is because it uses very long identifiers
and really maybe should just be left alone.

> This is my first patch.

Welcome, try again though.

If you really want to do something here maybe do something like
use temporaries to reduce line length and remove multiple
scnprintf statements.  This would also reduce object size.

---
 drivers/staging/gasket/apex_driver.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/staging/gasket/apex_driver.c b/drivers/staging/gasket/apex_driver.c
index 2973bb920a26..ae1a3a14dde6 100644
--- a/drivers/staging/gasket/apex_driver.c
+++ b/drivers/staging/gasket/apex_driver.c
@@ -509,6 +509,8 @@ static ssize_t sysfs_show(struct device *device, struct device_attribute *attr,
 	struct gasket_dev *gasket_dev;
 	struct gasket_sysfs_attribute *gasket_attr;
 	enum sysfs_attribute_type type;
+	struct gasket_page_table *gpt;
+	uint val;
 
 	gasket_dev = gasket_sysfs_get_device_data(device);
 	if (!gasket_dev) {
@@ -524,29 +526,27 @@ static ssize_t sysfs_show(struct device *device, struct device_attribute *attr,
 	}
 
 	type = (enum sysfs_attribute_type)gasket_attr->data.attr_type;
+	gpt = gasket_dev->page_table[0];
 	switch (type) {
 	case ATTR_KERNEL_HIB_PAGE_TABLE_SIZE:
-		ret = scnprintf(buf, PAGE_SIZE, "%u\n",
-				gasket_page_table_num_entries(
-					gasket_dev->page_table[0]));
+		val = gasket_page_table_num_entries(gpt);
 		break;
 	case ATTR_KERNEL_HIB_SIMPLE_PAGE_TABLE_SIZE:
-		ret = scnprintf(buf, PAGE_SIZE, "%u\n",
-				gasket_page_table_num_simple_entries(
-					gasket_dev->page_table[0]));
+		val = gasket_page_table_num_simple_entries(gpt);
 		break;
 	case ATTR_KERNEL_HIB_NUM_ACTIVE_PAGES:
-		ret = scnprintf(buf, PAGE_SIZE, "%u\n",
-				gasket_page_table_num_active_pages(
-					gasket_dev->page_table[0]));
+		val = gasket_page_table_num_active_pages(gpt);
 		break;
 	default:
 		dev_dbg(gasket_dev->dev, "Unknown attribute: %s\n",
 			attr->attr.name);
 		ret = 0;
-		break;
+		goto exit;
 	}
 
+	ret = scnprintf(buf, PAGE_SIZE, "%u\n", val);
+
+exit:
 	gasket_sysfs_put_attr(device, gasket_attr);
 	gasket_sysfs_put_device_data(device, gasket_dev);
 	return ret;


_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

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

* Re: [PATCH] Fixed parentheses malpractice in apex_driver.c
  2019-09-06 18:38 [PATCH] Fixed parentheses malpractice in apex_driver.c volery
  2019-09-06 20:55 ` Joe Perches
@ 2019-09-07 14:38 ` Dan Carpenter
  2019-09-07 14:48   ` Sandro Volery
  1 sibling, 1 reply; 5+ messages in thread
From: Dan Carpenter @ 2019-09-07 14:38 UTC (permalink / raw)
  To: volery; +Cc: devel, gregkh, linux-kernel, rspringer, toddpoynor

You need a subject prefix.  It should be something like:

[PATCH] Staging: gasket: Fix parentheses malpractice in apex_driver.c

Generally "Fix" is considered better style than "Fixed".  We aren't
going to care about that in staging, but the patch prefix is mandatory
so you will need to redo it anyway and might as well fix that as well.

On Fri, Sep 06, 2019 at 08:38:01PM +0200, volery wrote:
> There were some parentheses at the end of lines, which I took care of.
> This is my first patch.
  ^^^^^^^^^^^^^^^^^^^^^^

Put this sort of comments after the --- cut off line

> 
> Signed-off-by: Sandro Volery <sandro@volery.com>
> ---
  ^^^
Put it here.  It will be removed when we apply the patch so it won't
be recorded in the git log.

>  drivers/staging/gasket/apex_driver.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)

Joe's comments are, of course, correct as well.

regards,
dan carpenter

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

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

* Re: [PATCH] Fixed parentheses malpractice in apex_driver.c
  2019-09-07 14:38 ` Dan Carpenter
@ 2019-09-07 14:48   ` Sandro Volery
  2019-09-07 14:52     ` Dan Carpenter
  0 siblings, 1 reply; 5+ messages in thread
From: Sandro Volery @ 2019-09-07 14:48 UTC (permalink / raw)
  To: Dan Carpenter; +Cc: devel, gregkh, linux-kernel, rspringer, toddpoynor



> On 7 Sep 2019, at 16:39, Dan Carpenter <dan.carpenter@oracle.com> wrote:
> 
> You need a subject prefix.  It should be something like:
> 
> [PATCH] Staging: gasket: Fix parentheses malpractice in apex_driver.c
> 

Thanks for the reply! I'll try to do that better for my next patch.

> Generally "Fix" is considered better style than "Fixed".  We aren't
> going to care about that in staging, but the patch prefix is mandatory
> so you will need to redo it anyway and might as well fix that as well.
> 
>> On Fri, Sep 06, 2019 at 08:38:01PM +0200, volery wrote:
>> There were some parentheses at the end of lines, which I took care of.
>> This is my first patch.
>  ^^^^^^^^^^^^^^^^^^^^^^
> Put this sort of comments after the --- cut off line
> 
>> 
>> Signed-off-by: Sandro Volery <sandro@volery.com>
>> ---
>  ^^^
> Put it here.  It will be removed when we apply the patch so it won't
> be recorded in the git log.
> 

Alright :)

>> drivers/staging/gasket/apex_driver.c | 9 ++++++---
>> 1 file changed, 6 insertions(+), 3 deletions(-)
> 
> Joe's comments are, of course, correct as well.
> 
> regards,
> dan carpenter
> 

I'll take a look at Joe's suggestions too sometime tonight. I'd feel kinda bad tho if I just apply his work and send it in?

- Sandro Volery
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

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

* Re: [PATCH] Fixed parentheses malpractice in apex_driver.c
  2019-09-07 14:48   ` Sandro Volery
@ 2019-09-07 14:52     ` Dan Carpenter
  0 siblings, 0 replies; 5+ messages in thread
From: Dan Carpenter @ 2019-09-07 14:52 UTC (permalink / raw)
  To: Sandro Volery; +Cc: devel, gregkh, linux-kernel, rspringer, toddpoynor

On Sat, Sep 07, 2019 at 04:48:21PM +0200, Sandro Volery wrote:
> > Joe's comments are, of course, correct as well.
> > 
> > regards,
> > dan carpenter
> > 
> 
> I'll take a look at Joe's suggestions too sometime tonight. I'd feel kinda bad tho if I just apply his work and send it in?

Don't feel bad.  Just do it.  Give him credit in the commit message if
you want.  "Thanks to Joe Perches for his help."

I sometimes give people word for word patches and they don't copy it
exactly and I wonder if this is why...  My exact patch was the best one.

regards,
dan carpenter


_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

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

end of thread, back to index

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-06 18:38 [PATCH] Fixed parentheses malpractice in apex_driver.c volery
2019-09-06 20:55 ` Joe Perches
2019-09-07 14:38 ` Dan Carpenter
2019-09-07 14:48   ` Sandro Volery
2019-09-07 14:52     ` Dan Carpenter

DriverDev-Devel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/driverdev-devel/0 driverdev-devel/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 driverdev-devel driverdev-devel/ https://lore.kernel.org/driverdev-devel \
		driverdev-devel@linuxdriverproject.org devel@driverdev.osuosl.org driverdev-devel@archiver.kernel.org
	public-inbox-index driverdev-devel


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.linuxdriverproject.driverdev-devel


AGPL code for this site: git clone https://public-inbox.org/ public-inbox