Fix coding style in drivers/staging/crystalhd/crystalhd_lnx.c

PATCH Superseded Pascal COMBES 5 participants 17 messages Apr 13, 2014 – Apr 15, 2014 Staging

lore

Share & tools

lore

Pascal COMBES

author

2014-04-13 15:35 UTC #0 lore raw

PATCH 1/1 Fix coding style in drivers/staging/crystalhd/crystalhd_lnx.c

From: Pascal COMBES <[email protected]>

Fix alignement issues and two or three other coding style problems in drivers/staging/crystalhd/crystalhd_lnx.c.

NB: -I did this for task 10 of Eudyptula challenge <http://eudyptula-challenge.org/>
-I did not address camel case related problem because it would have make a big patch.

Signed-off-by Pascal COMBES <[email protected]>

---

drivers/staging/crystalhd/crystalhd_lnx.c +16 ~~-14~~

index 20be957..9f6a9d4 100644
@@ -28,7 +28,7 @@ static struct crystalhd_adp *g_adp_info;
 
 static irqreturn_t chd_dec_isr(int irq, void *arg)
 {
-	struct crystalhd_adp *adp = (struct crystalhd_adp *) arg;
+	struct crystalhd_adp *adp = (struct crystalhd_adp *)arg;
 	int rc = 0;
 	if (adp)
 		rc = crystalhd_cmd_interrupt(&adp->cmds);

@@ -112,7 +112,7 @@ static void chd_dec_free_iodata(struct crystalhd_adp *adp,
 }
 
 static inline int crystalhd_user_data(void __user *ud, void *dr,
-			 int size, int set)
+				      int size, int set)
 {
 	int rc;

@@ -135,7 +135,9 @@ static inline int crystalhd_user_data(void __user *ud, void *dr,
 }
 
 static int chd_dec_fetch_cdata(struct crystalhd_adp *adp,
-	 struct crystalhd_ioctl_data *io, uint32_t m_sz, unsigned long ua)
+			       struct crystalhd_ioctl_data *io,
+			       uint32_t m_sz,
+			       unsigned long ua)
 {
 	unsigned long ua_off;
 	int rc = 0;

@@ -154,7 +156,7 @@ static int chd_dec_fetch_cdata(struct crystalhd_adp *adp,
 	io->add_cdata_sz = m_sz;
 	ua_off = ua + sizeof(io->udata);
 	rc = crystalhd_user_data((void __user *)ua_off, io->add_cdata,
-			io->add_cdata_sz, 0);
+				 io->add_cdata_sz, 0);
 	if (rc) {
 		BCMLOG_ERR("failed to pull add_cdata sz:%x ua_off:%x\n",
 			   io->add_cdata_sz, (unsigned int)ua_off);

@@ -167,7 +169,8 @@ static int chd_dec_fetch_cdata(struct crystalhd_adp *adp,
 }
 
 static int chd_dec_release_cdata(struct crystalhd_adp *adp,
-			 struct crystalhd_ioctl_data *io, unsigned long ua)
+				 struct crystalhd_ioctl_data *io,
+				 unsigned long ua)
 {
 	unsigned long ua_off;
 	int rc;

@@ -180,7 +183,7 @@ static int chd_dec_release_cdata(struct crystalhd_adp *adp,
 	if (io->cmd != BCM_IOC_FW_DOWNLOAD) {
 		ua_off = ua + sizeof(io->udata);
 		rc = crystalhd_user_data((void __user *)ua_off, io->add_cdata,
-					io->add_cdata_sz, 1);
+					 io->add_cdata_sz, 1);
 		if (rc) {
 			BCMLOG_ERR(
 				"failed to push add_cdata sz:%x ua_off:%x\n",

@@ -210,7 +213,7 @@ static int chd_dec_proc_user_data(struct crystalhd_adp *adp,
 	}
 
 	rc = crystalhd_user_data((void __user *)ua, &io->udata,
-			sizeof(io->udata), set);
+				 sizeof(io->udata), set);
 	if (rc) {
 		BCMLOG_ERR("failed to %s iodata\n", (set ? "set" : "get"));
 		return rc;

@@ -382,7 +385,7 @@ static int chd_dec_init_chdev(struct crystalhd_adp *adp)
 	}
 
 	dev = device_create(crystalhd_class, NULL,
-			 MKDEV(adp->chd_dec_major, 0), NULL, "crystalhd");
+			    MKDEV(adp->chd_dec_major, 0), NULL, "crystalhd");
 	if (IS_ERR(dev)) {
 		rc = PTR_ERR(dev);
 		BCMLOG_ERR("failed to create device\n");

@@ -397,8 +400,7 @@ static int chd_dec_init_chdev(struct crystalhd_adp *adp)
 
 	/* Allocate general purpose ioctl pool. */
 	for (i = 0; i < CHD_IODATA_POOL_SZ; i++) {
-		temp = kzalloc(sizeof(struct crystalhd_ioctl_data),
-					 GFP_KERNEL);
+		temp = kzalloc(sizeof(*temp), GFP_KERNEL);
 		if (!temp) {
 			BCMLOG_ERR("ioctl data pool kzalloc failed\n");
 			rc = -ENOMEM;

@@ -549,11 +551,11 @@ static int chd_dec_pci_probe(struct pci_dev *pdev,
 	enum BC_STATUS sts = BC_STS_SUCCESS;
 
 	BCMLOG(BCMLOG_DBG,
-		"PCI_INFO: Vendor:0x%04x Device:0x%04x s_vendor:0x%04x s_device: 0x%04x\n",
-		pdev->vendor, pdev->device, pdev->subsystem_vendor,
-		pdev->subsystem_device);
+	       "PCI_INFO: Vendor:0x%04x Device:0x%04x s_vendor:0x%04x s_device: 0x%04x\n",
+	       pdev->vendor, pdev->device, pdev->subsystem_vendor,
+	       pdev->subsystem_device);
 
-	pinfo = kzalloc(sizeof(struct crystalhd_adp), GFP_KERNEL);
+	pinfo = kzalloc(sizeof(*pinfo), GFP_KERNEL);
 	if (!pinfo) {
 		BCMLOG_ERR("Failed to allocate memory\n");
 		return -ENOMEM;

Greg Kroah-Hartman

maintainer

2014-04-13 15:46 UTC #1 lore raw

Quoted 4 lines · Pascal COMBES

From: Pascal COMBES <[email protected]>
Fix alignement issues and two or three other coding style problems in drivers/staging/crystalhd/crystalhd_lnx.c.

Patches need to do one thing, as you are addressing a number of different things all in one patch, this needs to be broken up into smaller pieces, and sent as a series of patches.

Quoted 3 lines

NB: -I did this for task 10 of Eudyptula challenge <http://eudyptula-challenge.org/>
-I did not address camel case related problem because it would have make a big patch.

This doesn't belong here, in the patch changelog body, please remove it. If you want to include comments like this, they need to be below the "---" line where git will remove them when the patch is applied.

Quoted 26 lines

Signed-off-by Pascal COMBES <[email protected]>

---

drivers/staging/crystalhd/crystalhd_lnx.c +2 -2

index 20be957..9f6a9d4 100644
@@ -28,7 +28,7 @@ static struct crystalhd_adp *g_adp_info;
 
 static irqreturn_t chd_dec_isr(int irq, void *arg)
 {
-	struct crystalhd_adp *adp = (struct crystalhd_adp *) arg;
+	struct crystalhd_adp *adp = (struct crystalhd_adp *)arg;
 	int rc = 0;
 	if (adp)
 		rc = crystalhd_cmd_interrupt(&adp->cmds);

@@ -112,7 +112,7 @@ static void chd_dec_free_iodata(struct crystalhd_adp *adp,
 }
 
 static inline int crystalhd_user_data(void __user *ud, void *dr,
-			 int size, int set)
+				      int size, int set)
 {
 	int rc;

drivers/staging/crystalhd/crystalhd_lnx.c static inline int crystalhd_user_data(void __user *ud, void *dr, · line 140

Quoted 7 lines

drivers/staging/crystalhd/crystalhd_lnx.c +3 -1

@@ -135,7 +135,9 @@ static inline int crystalhd_user_data(void __user *ud, void *dr,
 }
 
 static int chd_dec_fetch_cdata(struct crystalhd_adp *adp,
-	 struct crystalhd_ioctl_data *io, uint32_t m_sz, unsigned long ua)
+			       struct crystalhd_ioctl_data *io,
+			       uint32_t m_sz,
+			       unsigned long ua)

Why can't these two lines be on the same line?

thanks,

greg k-h

Pascal COMBES

author

2014-04-13 19:09 UTC #2 lore raw

PATCH v2 1/3 Fix alignement problems in drivers/staging/crystalhd/crystalhd_lnx.c

From: Pascal COMBES <[email protected]>

Fix alignement problems in drivers/staging/crystalhd/crystalhd_lnx.c.

Signed-off-by Pascal COMBES <[email protected]>

---
As asked I split the patch in three (because I'm addressing 3 types of problems). I also took into account your last comment: What I did seemmed better to me, but as I'm not the reference I discarded it.
Regards,
Pascal COMBES.

drivers/staging/crystalhd/crystalhd_lnx.c +13 ~~-11~~

index 20be957..fd7f08a 100644
@@ -112,7 +112,7 @@ static void chd_dec_free_iodata(struct crystalhd_adp *adp,
 }
 
 static inline int crystalhd_user_data(void __user *ud, void *dr,
-			 int size, int set)
+				      int size, int set)
 {
 	int rc;

@@ -135,7 +135,8 @@ static inline int crystalhd_user_data(void __user *ud, void *dr,
 }
 
 static int chd_dec_fetch_cdata(struct crystalhd_adp *adp,
-	 struct crystalhd_ioctl_data *io, uint32_t m_sz, unsigned long ua)
+			       struct crystalhd_ioctl_data *io, uint32_t m_sz,
+			       unsigned long ua)
 {
 	unsigned long ua_off;
 	int rc = 0;

@@ -154,7 +155,7 @@ static int chd_dec_fetch_cdata(struct crystalhd_adp *adp,
 	io->add_cdata_sz = m_sz;
 	ua_off = ua + sizeof(io->udata);
 	rc = crystalhd_user_data((void __user *)ua_off, io->add_cdata,
-			io->add_cdata_sz, 0);
+				 io->add_cdata_sz, 0);
 	if (rc) {
 		BCMLOG_ERR("failed to pull add_cdata sz:%x ua_off:%x\n",
 			   io->add_cdata_sz, (unsigned int)ua_off);

@@ -167,7 +168,8 @@ static int chd_dec_fetch_cdata(struct crystalhd_adp *adp,
 }
 
 static int chd_dec_release_cdata(struct crystalhd_adp *adp,
-			 struct crystalhd_ioctl_data *io, unsigned long ua)
+				 struct crystalhd_ioctl_data *io,
+				 unsigned long ua)
 {
 	unsigned long ua_off;
 	int rc;

@@ -180,7 +182,7 @@ static int chd_dec_release_cdata(struct crystalhd_adp *adp,
 	if (io->cmd != BCM_IOC_FW_DOWNLOAD) {
 		ua_off = ua + sizeof(io->udata);
 		rc = crystalhd_user_data((void __user *)ua_off, io->add_cdata,
-					io->add_cdata_sz, 1);
+					 io->add_cdata_sz, 1);
 		if (rc) {
 			BCMLOG_ERR(
 				"failed to push add_cdata sz:%x ua_off:%x\n",

@@ -210,7 +212,7 @@ static int chd_dec_proc_user_data(struct crystalhd_adp *adp,
 	}
 
 	rc = crystalhd_user_data((void __user *)ua, &io->udata,
-			sizeof(io->udata), set);
+				 sizeof(io->udata), set);
 	if (rc) {
 		BCMLOG_ERR("failed to %s iodata\n", (set ? "set" : "get"));
 		return rc;

@@ -382,7 +384,7 @@ static int chd_dec_init_chdev(struct crystalhd_adp *adp)
 	}
 
 	dev = device_create(crystalhd_class, NULL,
-			 MKDEV(adp->chd_dec_major, 0), NULL, "crystalhd");
+			    MKDEV(adp->chd_dec_major, 0), NULL, "crystalhd");
 	if (IS_ERR(dev)) {
 		rc = PTR_ERR(dev);
 		BCMLOG_ERR("failed to create device\n");

@@ -398,7 +400,7 @@ static int chd_dec_init_chdev(struct crystalhd_adp *adp)
 	/* Allocate general purpose ioctl pool. */
 	for (i = 0; i < CHD_IODATA_POOL_SZ; i++) {
 		temp = kzalloc(sizeof(struct crystalhd_ioctl_data),
-					 GFP_KERNEL);
+			       GFP_KERNEL);
 		if (!temp) {
 			BCMLOG_ERR("ioctl data pool kzalloc failed\n");
 			rc = -ENOMEM;

@@ -549,9 +551,9 @@ static int chd_dec_pci_probe(struct pci_dev *pdev,
 	enum BC_STATUS sts = BC_STS_SUCCESS;
 
 	BCMLOG(BCMLOG_DBG,
-		"PCI_INFO: Vendor:0x%04x Device:0x%04x s_vendor:0x%04x s_device: 0x%04x\n",
-		pdev->vendor, pdev->device, pdev->subsystem_vendor,
-		pdev->subsystem_device);
+	       "PCI_INFO: Vendor:0x%04x Device:0x%04x s_vendor:0x%04x s_device: 0x%04x\n",
+	       pdev->vendor, pdev->device, pdev->subsystem_vendor,
+	       pdev->subsystem_device);
 
 	pinfo = kzalloc(sizeof(struct crystalhd_adp), GFP_KERNEL);
 	if (!pinfo) {

Dan Carpenter

2014-04-13 21:03 UTC #3 lore raw

Re: [PATCH v2 1/3] Fix alignement problems in drivers/staging/crystalhd/crystalhd_lnx.c

The subeject should be:

[PATCH v2 1/3] Staging: crystalhd: Fix alignement in crystalhd_lnx.c

Please resend it. Also you should use shorter subject lines.

"Fix coding style problem (cast with space) in drivers/staging/crystalhd/crystalhd_lnx.c"

This is too long. Say:

"Remove a stray space character in crystalhd_lnx.c"

Quoted 1 line · Pascal COMBES

From: Pascal COMBES <[email protected]>

Don't include this for your own patches, only when you are sending someone else's. We get the From information from the email header.

(This is harmless but it's bad style).

regards,
dan carpenter

Paul Bolle

2014-04-13 21:26 UTC #4 lore raw

Re: [PATCH v2 1/3] Fix alignement problems in drivers/staging/crystalhd/crystalhd_lnx.c

Quoted 11 lines · Dan Carpenter

The subeject should be:
[PATCH v2 1/3] Staging: crystalhd: Fix alignement in crystalhd_lnx.c
Please resend it. Also you should use shorter subject lines.
"Fix coding style problem (cast with space) in drivers/staging/crystalhd/crystalhd_lnx.c"
This is too long. Say:
"Remove a stray space character in crystalhd_lnx.c"

The commit explanation also was a copy of the subject line. That suggests that the explanation can be dropped. Many trivial fixes can (and should) be submitted without a commit explanation.

Paul Bolle

Dan Carpenter

2014-04-13 22:07 UTC #5 lore raw

Re: [PATCH v2 1/3] Fix alignement problems in drivers/staging/crystalhd/crystalhd_lnx.c

Quoted 7 lines · Paul Bolle

Quoted 6 lines · Dan Carpenter
The subeject should be:
[PATCH v2 1/3] Staging: crystalhd: Fix alignement in crystalhd_lnx.c
Please resend it. Also you should use shorter subject lines.

Quoted 7 lines

Quoted 5 lines
"Fix coding style problem (cast with space) in drivers/staging/crystalhd/crystalhd_lnx.c"
This is too long. Say:
"Remove a stray space character in crystalhd_lnx.c"
The commit explanation also was a copy of the subject line.

Huh? No it wasn't.

Quoted 2 lines

That
suggests that the explanation can be dropped.

The long description was fine. It included a copy of the error message and everything.

Quoted 3 lines

Many trivial fixes can
(and should) be submitted without a commit explanation.

Don't encourage people to leave out the commit message. They can figure out on their own when that is appropriate. In my experience people only err on the other side, by not commenting enough. I've only rarely been annoyed by a long description which was too long.

regards,
dan carpenter

Paul Bolle

2014-04-13 22:13 UTC #6 lore raw

Re: [PATCH v2 1/3] Fix alignement problems in drivers/staging/crystalhd/crystalhd_lnx.c

Quoted 2 lines · Dan Carpenter

The long description was fine. It included a copy of the error message and everything.

This is what I saw in the body of the message above the "---" line:
From: Pascal COMBES <[email protected]>

Fix alignement problems in drivers/staging/crystalhd/crystalhd_lnx.c.

Signed-off-by Pascal COMBES <[email protected]>

Are you and I looking at different patches?

Paul Bolle

Dan Carpenter

2014-04-13 22:46 UTC #7 lore raw

Re: [PATCH v2 1/3] Fix alignement problems in drivers/staging/crystalhd/crystalhd_lnx.c

Quoted 4 lines · Paul Bolle

Quoted 2 lines · Dan Carpenter
The long description was fine. It included a copy of the error message and everything.

Quoted 8 lines

This is what I saw in the body of the message above the "---" line:
From: Pascal COMBES <[email protected]>
Fix alignement problems in drivers/staging/crystalhd/crystalhd_lnx.c.
Signed-off-by Pascal COMBES <[email protected]>
Are you and I looking at different patches?

Yeah. I was looking at patch 2 where the space was removed.

regards,
dan carpenter

Pascal COMBES

author

2014-04-13 19:13 UTC #8 lore raw

PATCH v2 2/3 Fix coding style problem (cast with space) in drivers/staging/crystalhd/crystalhd_lnx.c

In reply to Greg Kroah-Hartman

From: Pascal COMBES <[email protected]>

Fix coding style problem in drivers/staging/crystalhd/crystalhd_lnx.c:
No space needed before a cast.

Signed-off-by Pascal COMBES <[email protected]>

---

drivers/staging/crystalhd/crystalhd_lnx.c +1 -1

index fd7f08a..15e8f02 100644
@@ -28,7 +28,7 @@ static struct crystalhd_adp *g_adp_info;
 
 static irqreturn_t chd_dec_isr(int irq, void *arg)
 {
-	struct crystalhd_adp *adp = (struct crystalhd_adp *) arg;
+	struct crystalhd_adp *adp = (struct crystalhd_adp *)arg;
 	int rc = 0;
 	if (adp)
 		rc = crystalhd_cmd_interrupt(&adp->cmds);

Paul Bolle

2014-04-13 21:36 UTC #9 lore raw

Re: [PATCH v2 2/3] Fix coding style problem (cast with space) in drivers/staging/crystalhd/crystalhd_lnx.c

Quoted 12 lines · Pascal COMBES

From: Pascal COMBES <[email protected]>
Fix coding style problem in drivers/staging/crystalhd/crystalhd_lnx.c:
No space needed before a cast.
Signed-off-by Pascal COMBES <[email protected]>
---
drivers/staging/crystalhd/crystalhd_lnx.c +0 -0
index fd7f08a..15e8f02 100644

drivers/staging/crystalhd/crystalhd_lnx.c static struct crystalhd_adp *g_adp_info; · line 31

Quoted 5 lines

drivers/staging/crystalhd/crystalhd_lnx.c +1 -1

@@ -28,7 +28,7 @@ static struct crystalhd_adp *g_adp_info;
 
 static irqreturn_t chd_dec_isr(int irq, void *arg)
 {
-	struct crystalhd_adp *adp = (struct crystalhd_adp *) arg;
+	struct crystalhd_adp *adp = (struct crystalhd_adp *)arg;

Shouldn't this cast just be dropped instead?

drivers/staging/crystalhd/crystalhd_lnx.c static struct crystalhd_adp *g_adp_info; · line 34

Quoted 4 lines

drivers/staging/crystalhd/crystalhd_lnx.c +0 -0
 	int rc = 0;
 	if (adp)
 		rc = crystalhd_cmd_interrupt(&adp->cmds);

Paul Bolle

Pascal COMBES

author

2014-04-14 16:46 UTC #10 lore raw

Re: [PATCH v2 2/3] Fix coding style problem (cast with space) in drivers/staging/crystalhd/crystalhd_lnx.c

Quoted 14 lines · Paul Bolle

Quoted 13 lines · Pascal COMBES
From: Pascal COMBES <[email protected]>
Fix coding style problem in drivers/staging/crystalhd/crystalhd_lnx.c:
No space needed before a cast.
Signed-off-by Pascal COMBES <[email protected]>
---
drivers/staging/crystalhd/crystalhd_lnx.c +0 -0
index fd7f08a..15e8f02 100644
@@ -28,7 +28,7 @@ static struct crystalhd_adp *g_adp_info;
 

Quoted 6 lines

drivers/staging/crystalhd/crystalhd_lnx.c static struct crystalhd_adp *g_adp_info; · line 31
Quoted 4 lines
drivers/staging/crystalhd/crystalhd_lnx.c +1 -1
 static irqreturn_t chd_dec_isr(int irq, void *arg)
 {
-	struct crystalhd_adp *adp = (struct crystalhd_adp *) arg;
+	struct crystalhd_adp *adp = (struct crystalhd_adp *)arg;
Shouldn't this cast just be dropped instead?

I think as you and was wondering why it was there but I didn't dare removing it. I considered it was not really coding style. But after second thought, I will remove it when I resend tomrorrow.
Regards,
Pascal COMBES.

Quoted 10 lines

drivers/staging/crystalhd/crystalhd_lnx.c static struct crystalhd_adp *g_adp_info; · line 34
Quoted 5 lines
drivers/staging/crystalhd/crystalhd_lnx.c +0 -0
 	int rc = 0;
 	if (adp)
 		rc = crystalhd_cmd_interrupt(&adp->cmds);
Paul Bolle

Jingoo Han

2014-04-15 00:44 UTC #11 lore raw

Re: [PATCH v2 2/3] Fix coding style problem (cast with space) in drivers/staging/crystalhd/crystalhd_lnx.c

Quoted 23 lines · Pascal COMBES

Quoted 14 lines · Paul Bolle
Quoted 13 lines · Pascal COMBES
From: Pascal COMBES <[email protected]>
Fix coding style problem in drivers/staging/crystalhd/crystalhd_lnx.c:
No space needed before a cast.
Signed-off-by Pascal COMBES <[email protected]>
---
drivers/staging/crystalhd/crystalhd_lnx.c +0 -0
index fd7f08a..15e8f02 100644
@@ -28,7 +28,7 @@ static struct crystalhd_adp *g_adp_info;
 
Quoted 6 lines
drivers/staging/crystalhd/crystalhd_lnx.c static struct crystalhd_adp *g_adp_info; · line 31
Quoted 4 lines
drivers/staging/crystalhd/crystalhd_lnx.c +1 -1
 static irqreturn_t chd_dec_isr(int irq, void *arg)
 {
-	struct crystalhd_adp *adp = (struct crystalhd_adp *) arg;
+	struct crystalhd_adp *adp = (struct crystalhd_adp *)arg;
Shouldn't this cast just be dropped instead?

Quoted 3 lines

I think as you and was wondering why it was there but I didn't dare removing it. I considered it was not really coding style. But after second thought, I will remove it when I resend tomrorrow.

Please, remove unnecessary void cast as below.

- struct crystalhd_adp *adp = (struct crystalhd_adp *) arg;
+ struct crystalhd_adp *adp = arg;

This is because the conversion from void pointer to any other pointer type is guaranteed by the C programming language. So, void cast is unnecessary.

Best regards,
Jingoo Han

Quoted 12 lines

Regards,
Pascal COMBES.
Quoted 10 lines
drivers/staging/crystalhd/crystalhd_lnx.c static struct crystalhd_adp *g_adp_info; · line 34
Quoted 5 lines
drivers/staging/crystalhd/crystalhd_lnx.c +0 -0
 	int rc = 0;
 	if (adp)
 		rc = crystalhd_cmd_interrupt(&adp->cmds);
Paul Bolle

Pascal COMBES

author

2014-04-13 19:13 UTC #12 lore raw

PATCH v2 3/3 Fix coding style problem (sizeof with type) in drivers/staging/crystalhd/crystalhd_lnx.c

In reply to Greg Kroah-Hartman

From: Pascal COMBES <[email protected]>

Replace sizeof(type) by sizeof(variable) in
drivers/staging/crystalhd/crystalhd_lnx.c.

Signed-off-by Pascal COMBES <[email protected]>

---

(quoted diff) +0 -0

b/drivers/staging/crystalhd/crystalhd_lnx.c
index 15e8f02..7b14b28 100644

drivers/staging/crystalhd/crystalhd_lnx.c +2 -3

@@ -399,8 +399,7 @@ static int chd_dec_init_chdev(struct crystalhd_adp *adp)
 
 	/* Allocate general purpose ioctl pool. */
 	for (i = 0; i < CHD_IODATA_POOL_SZ; i++) {
-		temp = kzalloc(sizeof(struct crystalhd_ioctl_data),
-			       GFP_KERNEL);
+		temp = kzalloc(sizeof(*temp), GFP_KERNEL);
 		if (!temp) {
 			BCMLOG_ERR("ioctl data pool kzalloc failed\n");
 			rc = -ENOMEM;

@@ -555,7 +554,7 @@ static int chd_dec_pci_probe(struct pci_dev *pdev,
 	       pdev->vendor, pdev->device, pdev->subsystem_vendor,
 	       pdev->subsystem_device);
 
-	pinfo = kzalloc(sizeof(struct crystalhd_adp), GFP_KERNEL);
+	pinfo = kzalloc(sizeof(*pinfo), GFP_KERNEL);
 	if (!pinfo) {
 		BCMLOG_ERR("Failed to allocate memory\n");
 		return -ENOMEM;

Pascal COMBES

author

2014-04-15 19:21 UTC #13 lore raw

PATCH v3 1/3 Staging: crystalhd: Fix alignement in crystalhd_lnx.c

In reply to Greg Kroah-Hartman

Signed-off-by Pascal COMBES <[email protected]>

---
Description in subject.

drivers/staging/crystalhd/crystalhd_lnx.c +13 ~~-11~~

index 20be957..fd7f08a 100644
@@ -112,7 +112,7 @@ static void chd_dec_free_iodata(struct crystalhd_adp *adp,
 }
 
 static inline int crystalhd_user_data(void __user *ud, void *dr,
-			 int size, int set)
+				      int size, int set)
 {
 	int rc;

@@ -135,7 +135,8 @@ static inline int crystalhd_user_data(void __user *ud, void *dr,
 }
 
 static int chd_dec_fetch_cdata(struct crystalhd_adp *adp,
-	 struct crystalhd_ioctl_data *io, uint32_t m_sz, unsigned long ua)
+			       struct crystalhd_ioctl_data *io, uint32_t m_sz,
+			       unsigned long ua)
 {
 	unsigned long ua_off;
 	int rc = 0;

@@ -154,7 +155,7 @@ static int chd_dec_fetch_cdata(struct crystalhd_adp *adp,
 	io->add_cdata_sz = m_sz;
 	ua_off = ua + sizeof(io->udata);
 	rc = crystalhd_user_data((void __user *)ua_off, io->add_cdata,
-			io->add_cdata_sz, 0);
+				 io->add_cdata_sz, 0);
 	if (rc) {
 		BCMLOG_ERR("failed to pull add_cdata sz:%x ua_off:%x\n",
 			   io->add_cdata_sz, (unsigned int)ua_off);

@@ -167,7 +168,8 @@ static int chd_dec_fetch_cdata(struct crystalhd_adp *adp,
 }
 
 static int chd_dec_release_cdata(struct crystalhd_adp *adp,
-			 struct crystalhd_ioctl_data *io, unsigned long ua)
+				 struct crystalhd_ioctl_data *io,
+				 unsigned long ua)
 {
 	unsigned long ua_off;
 	int rc;

@@ -180,7 +182,7 @@ static int chd_dec_release_cdata(struct crystalhd_adp *adp,
 	if (io->cmd != BCM_IOC_FW_DOWNLOAD) {
 		ua_off = ua + sizeof(io->udata);
 		rc = crystalhd_user_data((void __user *)ua_off, io->add_cdata,
-					io->add_cdata_sz, 1);
+					 io->add_cdata_sz, 1);
 		if (rc) {
 			BCMLOG_ERR(
 				"failed to push add_cdata sz:%x ua_off:%x\n",

@@ -210,7 +212,7 @@ static int chd_dec_proc_user_data(struct crystalhd_adp *adp,
 	}
 
 	rc = crystalhd_user_data((void __user *)ua, &io->udata,
-			sizeof(io->udata), set);
+				 sizeof(io->udata), set);
 	if (rc) {
 		BCMLOG_ERR("failed to %s iodata\n", (set ? "set" : "get"));
 		return rc;

@@ -382,7 +384,7 @@ static int chd_dec_init_chdev(struct crystalhd_adp *adp)
 	}
 
 	dev = device_create(crystalhd_class, NULL,
-			 MKDEV(adp->chd_dec_major, 0), NULL, "crystalhd");
+			    MKDEV(adp->chd_dec_major, 0), NULL, "crystalhd");
 	if (IS_ERR(dev)) {
 		rc = PTR_ERR(dev);
 		BCMLOG_ERR("failed to create device\n");

@@ -398,7 +400,7 @@ static int chd_dec_init_chdev(struct crystalhd_adp *adp)
 	/* Allocate general purpose ioctl pool. */
 	for (i = 0; i < CHD_IODATA_POOL_SZ; i++) {
 		temp = kzalloc(sizeof(struct crystalhd_ioctl_data),
-					 GFP_KERNEL);
+			       GFP_KERNEL);
 		if (!temp) {
 			BCMLOG_ERR("ioctl data pool kzalloc failed\n");
 			rc = -ENOMEM;

@@ -549,9 +551,9 @@ static int chd_dec_pci_probe(struct pci_dev *pdev,
 	enum BC_STATUS sts = BC_STS_SUCCESS;
 
 	BCMLOG(BCMLOG_DBG,
-		"PCI_INFO: Vendor:0x%04x Device:0x%04x s_vendor:0x%04x s_device: 0x%04x\n",
-		pdev->vendor, pdev->device, pdev->subsystem_vendor,
-		pdev->subsystem_device);
+	       "PCI_INFO: Vendor:0x%04x Device:0x%04x s_vendor:0x%04x s_device: 0x%04x\n",
+	       pdev->vendor, pdev->device, pdev->subsystem_vendor,
+	       pdev->subsystem_device);
 
 	pinfo = kzalloc(sizeof(struct crystalhd_adp), GFP_KERNEL);
 	if (!pinfo) {

Dan Carpenter

2014-04-15 19:33 UTC #14 lore raw

Re: [PATCH v3 1/3] Staging: crystalhd: Fix alignement in crystalhd_lnx.c

All three look great now.

regards,
dan carpenter

Pascal COMBES

author

2014-04-15 19:21 UTC #15 lore raw

PATCH v3 2/3 Staging: crystalhd: Removed cast in crystalhd_lnx.c

In reply to Greg Kroah-Hartman

Removed useless (because automatic) cast from void * in crystalhd_lnx.c

Signed-off-by Pascal COMBES <[email protected]>

---

drivers/staging/crystalhd/crystalhd_lnx.c +1 -1

index fd7f08a..0e432bc 100644
@@ -28,7 +28,7 @@ static struct crystalhd_adp *g_adp_info;
 
 static irqreturn_t chd_dec_isr(int irq, void *arg)
 {
-	struct crystalhd_adp *adp = (struct crystalhd_adp *) arg;
+	struct crystalhd_adp *adp = arg;
 	int rc = 0;
 	if (adp)
 		rc = crystalhd_cmd_interrupt(&adp->cmds);

Pascal COMBES

author

2014-04-15 19:21 UTC #16 lore raw

PATCH v3 3/3 Staging: crystalhd: Improve kzalloc calls in crystalhd_lnx.c

In reply to Greg Kroah-Hartman

Replace sizeof(type) by sizeof(variable) in crystalhd_lnx.c.

Signed-off-by Pascal COMBES <[email protected]>

---

drivers/staging/crystalhd/crystalhd_lnx.c +2 -3

index 0e432bc..e6fb331 100644
@@ -399,8 +399,7 @@ static int chd_dec_init_chdev(struct crystalhd_adp *adp)
 
 	/* Allocate general purpose ioctl pool. */
 	for (i = 0; i < CHD_IODATA_POOL_SZ; i++) {
-		temp = kzalloc(sizeof(struct crystalhd_ioctl_data),
-			       GFP_KERNEL);
+		temp = kzalloc(sizeof(*temp), GFP_KERNEL);
 		if (!temp) {
 			BCMLOG_ERR("ioctl data pool kzalloc failed\n");
 			rc = -ENOMEM;

@@ -555,7 +554,7 @@ static int chd_dec_pci_probe(struct pci_dev *pdev,
 	       pdev->vendor, pdev->device, pdev->subsystem_vendor,
 	       pdev->subsystem_device);
 
-	pinfo = kzalloc(sizeof(struct crystalhd_adp), GFP_KERNEL);
+	pinfo = kzalloc(sizeof(*pinfo), GFP_KERNEL);
 	if (!pinfo) {
 		BCMLOG_ERR("Failed to allocate memory\n");
 		return -ENOMEM;