Skip to contentSkip to conversation

edac: pnd2_edac: add code comment for clarification

PATCH In review Gustavo A. R. Silva 2 participants 6 messages Jun 22, 2017 – Jun 23, 2017

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH 1/2 edac: pnd2_edac: add code comment for clarification

Add code comment to make it clear that the fall-through is intentional.

Addresses-Coverity-ID: 1403728

---
 drivers/edac/pnd2_edac.c | 1 +
 1 file changed, 1 insertion(+)
drivers/edac/pnd2_edac.c +1 -0
index 1cad5a9..c51ec73 100644
@@ -174,6 +174,7 @@ static int apl_rd_reg(int port, int off, int op, void *data, size_t sz, char *na
 	switch (sz) {
 	case 8:
 		ret = sbi_send(port, off + 4, op, (u32 *)(data + 4));
+		/* fall through */
 	case 4:
 		ret = sbi_send(port, off, op, (u32 *)data);
 		pnd2_printk(KERN_DEBUG, "%s=%x%08x ret=%d\n", name,
#1 lore raw

PATCH 2/2 edac: pnd2_edac: remove useless variable assignment

Value assigned to variable _ret_ at line 176 is overwritten a few lines below before it can be used. This makes such variable assignment useless.

Addresses-Coverity-ID: 1403730

---
 drivers/edac/pnd2_edac.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/edac/pnd2_edac.c +1 -1
index c51ec73..d1de53e 100644
@@ -173,7 +173,7 @@ static int apl_rd_reg(int port, int off, int op, void *data, size_t sz, char *na
 	edac_dbg(2, "Read %s port=%x off=%x op=%x\n", name, port, off, op);
 	switch (sz) {
 	case 8:
-		ret = sbi_send(port, off + 4, op, (u32 *)(data + 4));
+		sbi_send(port, off + 4, op, (u32 *)(data + 4));
 		/* fall through */
 	case 4:
 		ret = sbi_send(port, off, op, (u32 *)data);
maintainer
#2 lore raw

Re: [PATCH 2/2] edac: pnd2_edac: remove useless variable assignment

Quoted 5 lines · Gustavo A. R. Silva

Value assigned to variable _ret_ at line 176 is overwritten a few lines below before it can be used. This makes such variable assignment useless.

Addresses-Coverity-ID: 1403730

Get rid of this tag...

Quoted 10 lines
---
 drivers/edac/pnd2_edac.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/edac/pnd2_edac.c +0 -0
index c51ec73..d1de53e 100644

drivers/edac/pnd2_edac.c static int apl_rd_reg(int port, int off, int op, void *data, size_t sz, char *na · line 176

Quoted 5 lines
drivers/edac/pnd2_edac.c +1 -1
@@ -173,7 +173,7 @@ static int apl_rd_reg(int port, int off, int op, void *data, size_t sz, char *na
 	edac_dbg(2, "Read %s port=%x off=%x op=%x\n", name, port, off, op);
 	switch (sz) {
 	case 8:
-		ret = sbi_send(port, off + 4, op, (u32 *)(data + 4));
+		sbi_send(port, off + 4, op, (u32 *)(data + 4));

... keep ret but OR it in with its previous value:

        case 8:
                ret  = sbi_send(port, off + 4, op, (u32 *)(data + 4));
		/* fall through */
        case 4:
                ret |= sbi_send(port, off, op, (u32 *)data);

drivers/edac/pnd2_edac.c static int apl_rd_reg(int port, int off, int op, void *data, size_t sz, char *na · line 179

Quoted 3 lines
drivers/edac/pnd2_edac.c +0 -0
 		/* fall through */
 	case 4:
 		ret = sbi_send(port, off, op, (u32 *)data);

... and merge both into a single patch.

Thanks.

#3 lore raw

Re: [PATCH 2/2] edac: pnd2_edac: remove useless variable assignment

Hi Borislav,

Quoting Borislav Petkov <[email protected]>:

Quoted 34 lines · Gustavo A. R. Silva
Quoted 5 lines

Value assigned to variable _ret_ at line 176 is overwritten a few lines below before it can be used. This makes such variable assignment useless.

Addresses-Coverity-ID: 1403730

Get rid of this tag...

Quoted 11 lines
---
 drivers/edac/pnd2_edac.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/edac/pnd2_edac.c +0 -0
index c51ec73..d1de53e 100644
@@ -173,7 +173,7 @@ static int apl_rd_reg(int port, int off, int  

op, void *data, size_t sz, char *na

drivers/edac/pnd2_edac.c static int apl_rd_reg(int port, int off, int op, void *data, size_t sz, char *na · line 176

Quoted 5 lines
drivers/edac/pnd2_edac.c +1 -1
 	edac_dbg(2, "Read %s port=%x off=%x op=%x\n", name, port, off, op);
 	switch (sz) {
 	case 8:
-		ret = sbi_send(port, off + 4, op, (u32 *)(data + 4));
+		sbi_send(port, off + 4, op, (u32 *)(data + 4));

... keep ret but OR it in with its previous value:

        case 8:
                ret  = sbi_send(port, off + 4, op, (u32 *)(data + 4));
		/* fall through */
        case 4:
                ret |= sbi_send(port, off, op, (u32 *)data);
Quoted 6 lines

drivers/edac/pnd2_edac.c static int apl_rd_reg(int port, int off, int op, void *data, size_t sz, char *na · line 179

Quoted 3 lines
drivers/edac/pnd2_edac.c +0 -0
 		/* fall through */
 	case 4:
 		ret = sbi_send(port, off, op, (u32 *)data);

... and merge both into a single patch.

I got it. I'll send a new patch shortly.

Thanks!
---
Gustavo A. R. Silva

--
To unsubscribe from this list: send the line "unsubscribe linux-edac" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

#4 lore raw

PATCH 1/1 edac: pnd2_edac: add code comment for clarification and update variable assignment

Add code comment to make it clear that the fall-through is intentional and, OR ret with its previous value to avoid overwriting it.

---
 drivers/edac/pnd2_edac.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
drivers/edac/pnd2_edac.c +2 -1
index 1cad5a9..63cb323 100644
@@ -174,8 +174,9 @@ static int apl_rd_reg(int port, int off, int op, void *data, size_t sz, char *na
 	switch (sz) {
 	case 8:
 		ret = sbi_send(port, off + 4, op, (u32 *)(data + 4));
+		/* fall through */
 	case 4:
-		ret = sbi_send(port, off, op, (u32 *)data);
+		ret |= sbi_send(port, off, op, (u32 *)data);
 		pnd2_printk(KERN_DEBUG, "%s=%x%08x ret=%d\n", name,
 					sz == 8 ? *((u32 *)(data + 4)) : 0, *((u32 *)data), ret);
 		break;
maintainer Applied
#5 lore raw

Re: [PATCH] edac: pnd2_edac: add code comment for clarification and update variable assignment

Quoted 8 lines · Gustavo A. R. Silva

Add code comment to make it clear that the fall-through is intentional and, OR ret with its previous value to avoid overwriting it.

---
 drivers/edac/pnd2_edac.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Applied, thanks.