Skip to contentSkip to conversation

Staging: wlan-ng: Handle error condition.

PATCH Superseded Sandhya Bankar 2 participants 5 messages Feb 28, 2016 Staging

v1 of 3 Superseded Latest is v3 →

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 Staging: wlan-ng: Handle error condition.

Handle error condition.
Below semantic patch found,their is double assignment to 'result' variable.so instead of deleting double assignment,handling error condition for "result = mkimage(..)". @@
expression e1,e2,e3;
@@

(
(<+...e1++...+>)=e2;
|
(<+...e1--...+>)=e2;
|
(<+...++e1...+>)=e2;
|
(<+...--e1...+>)=e2;
|
e1=e2;
e1 = <+...e1...+>;
|
*e1=e2;
*e1=e3;

---
 drivers/staging/wlan-ng/prism2fw.c | 4 ++++
 1 file changed, 4 insertions(+)
drivers/staging/wlan-ng/prism2fw.c +4 -0
index 8fc80df..35fd5fb 100644
@@ -333,6 +333,10 @@ static int prism2_fwapply(const struct ihex_binrec *rfptr,
 
 	/* Make the image chunks */
 	result = mkimage(fchunk, &nfchunks);
+	if (result) {
+		netdev_err(wlandev->netdev, "Failed to make image chunk.\n");
+		return 1;
+	}
 
 	/* Do any plugging */
 	result = plugimage(fchunk, nfchunks, s3plug, ns3plug, &pda);
Signature · 1 line
1.8.3.4
#1 lore raw
Quoted 31 lines · Sandhya Bankar

Handle error condition.
Below semantic patch found,their is double assignment to 'result' variable.so instead of deleting double assignment,handling error condition for "result = mkimage(..)". @@
expression e1,e2,e3;
@@

(
(<+...e1++...+>)=e2;
|
(<+...e1--...+>)=e2;
|
(<+...++e1...+>)=e2;
|
(<+...--e1...+>)=e2;
|
e1=e2;
e1 = <+...e1...+>;
|
*e1=e2;
*e1=e3;

---
 drivers/staging/wlan-ng/prism2fw.c | 4 ++++
 1 file changed, 4 insertions(+)
drivers/staging/wlan-ng/prism2fw.c +0 -0
index 8fc80df..35fd5fb 100644

drivers/staging/wlan-ng/prism2fw.c static int prism2_fwapply(const struct ihex_binrec *rfptr, · line 339

Quoted 7 lines
drivers/staging/wlan-ng/prism2fw.c +4 -0
@@ -333,6 +333,10 @@ static int prism2_fwapply(const struct ihex_binrec *rfptr,
 
 	/* Make the image chunks */
 	result = mkimage(fchunk, &nfchunks);
+	if (result) {
+		netdev_err(wlandev->netdev, "Failed to make image chunk.\n");
+		return 1;
+	}

Perhaps it would be possible to move this code away from using 1 for failure, towards using actual error codes.

julia

Quoted 13 lines
drivers/staging/wlan-ng/prism2fw.c +0 -0
 	/* Do any plugging */
 	result = plugimage(fchunk, nfchunks, s3plug, ns3plug, &pda);
Signature · 8 lines
1.8.3.4

-- 
You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [email protected].
To post to this group, send email to [email protected].
To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/20160228033629.GA7898%40sandhya.
For more options, visit https://groups.google.com/d/optout.
#2 lore raw

The mkimage(..) is returning error i.e return 1 from below error code path, First error condition is if kzalloc(..) has failed,then return 1. Second error condition is if chunk mismatch then it return 1.

I also checked the prism2_fwapply(..) in every error condition,it is returning 1,so i have used return 1 in above patch.

Do you have any suggestion ?

Quoted 71 lines · Julia Lawall
Quoted 2 lines · Sandhya Bankar

Handle error condition.
Below semantic patch found,their is double assignment to 'result'

variable.so instead of deleting double assignment,handling error condition for "result = mkimage(..)".

Quoted 19 lines

@@
expression e1,e2,e3;
@@

(
(<+...e1++...+>)=e2;
|
(<+...e1--...+>)=e2;
|
(<+...++e1...+>)=e2;
|
(<+...--e1...+>)=e2;
|
e1=e2;
e1 = <+...e1...+>;
|
*e1=e2;
*e1=e3;

Review

Quoted 6 lines
---
 drivers/staging/wlan-ng/prism2fw.c | 4 ++++
 1 file changed, 4 insertions(+)
(quoted diff) +0 -0

b/drivers/staging/wlan-ng/prism2fw.c

drivers/staging/wlan-ng/prism2fw.c static int prism2_fwapply(const struct ihex_binrec

Quoted 4 lines

index 8fc80df..35fd5fb 100644

drivers/staging/wlan-ng/prism2fw.c +0 -0
@@ -333,6 +333,10 @@ static int prism2_fwapply(const struct ihex_binrec

*rfptr,

drivers/staging/wlan-ng/prism2fw.c static int prism2_fwapply(const struct ihex_binrec *rfptr,

Quoted 5 lines
drivers/staging/wlan-ng/prism2fw.c +2 -0
      /* Make the image chunks */
      result = mkimage(fchunk, &nfchunks);
+     if (result) {
+             netdev_err(wlandev->netdev, "Failed to make image

chunk.\n");

drivers/staging/wlan-ng/prism2fw.c static int prism2_fwapply(const struct ihex_binrec *rfptr, · line 339

Quoted 2 lines
drivers/staging/wlan-ng/prism2fw.c +2 -0
+             return 1;
+     }

Perhaps it would be possible to move this code away from using 1 for failure, towards using actual error codes.

julia

Quoted 8 lines
drivers/staging/wlan-ng/prism2fw.c +0 -1
      /* Do any plugging */
      result = plugimage(fchunk, nfchunks, s3plug, ns3plug, &pda);
--

1.8.3.4

--
You received this message because you are subscribed to the Google

Groups "outreachy-kernel" group.

Quoted 1 line

To unsubscribe from this group and stop receiving emails from it, send

an email to [email protected].

Quoted 2 lines

To post to this group, send email to [email protected]. To view this discussion on the web visit

https://groups.google.com/d/msgid/outreachy-kernel/20160228033629.GA7898%40sandhya
.

Quoted 2 lines

For more options, visit https://groups.google.com/d/optout.

#3 lore raw
Quoted 6 lines · sandhya bankar

The mkimage(..) is returning error i.e return 1 from below error code path, First error condition is if kzalloc(..) has failed,then return 1. Second error condition is if chunk mismatch then it return 1.

I also checked the prism2_fwapply(..) in every error condition,it is returning 1,so i have used return 1 in above patch.

Yes, I see that they are returning 1 everywhere, so there is no problem with the patch. But the whole code should be improved to return proper error codes, like -ENOMEM on allocation failure, rather than 1. There are even some comments in the code saying that typically an error code is returned on failure, but that is not the case. 1 indeed looks more like success (true).

julia

Quoted 79 lines

Do you have any suggestion ?

On Sun, Feb 28, 2016 at 5:31 PM, Julia Lawall <[email protected]> wrote:

      On Sun, 28 Feb 2016, Sandhya Bankar wrote:

      > Handle error condition.
      > Below semantic patch found,their is double assignment to
      'result' variable.so instead of deleting double
      assignment,handling error condition for "result = mkimage(..)".
      > @@
      > expression e1,e2,e3;
      > @@
      >
      > (
      >  (<+...e1++...+>)=e2;
      > |
      >  (<+...e1--...+>)=e2;
      > |
      >  (<+...++e1...+>)=e2;
      > |
      >  (<+...--e1...+>)=e2;
      > |
      > e1=e2;
      > e1 = <+...e1...+>;
      > |
      > *e1=e2;
      > *e1=e3;
      >
      > Signed-off-by: Sandhya Bankar <[email protected]>
      > ---
      >  drivers/staging/wlan-ng/prism2fw.c | 4 ++++
      >  1 file changed, 4 insertions(+)
      >
      > diff --git a/drivers/staging/wlan-ng/prism2fw.c
      b/drivers/staging/wlan-ng/prism2fw.c
      > index 8fc80df..35fd5fb 100644
      > --- a/drivers/staging/wlan-ng/prism2fw.c
      > +++ b/drivers/staging/wlan-ng/prism2fw.c
      > @@ -333,6 +333,10 @@ static int prism2_fwapply(const struct
      ihex_binrec *rfptr,
      >
      >       /* Make the image chunks */
      >       result = mkimage(fchunk, &nfchunks);
      > +     if (result) {
      > +             netdev_err(wlandev->netdev, "Failed to make
      image chunk.\n");
      > +             return 1;
      > +     }

Perhaps it would be possible to move this code away from using 1 for failure, towards using actual error codes.

julia

Quoted 8 lines

       /* Do any plugging */
       result = plugimage(fchunk, nfchunks, s3plug, ns3plug, &pda); --
1.8.3.4

--
You received this message because you are subscribed to the Google

Groups "outreachy-kernel" group.

Quoted 1 line

To unsubscribe from this group and stop receiving emails from it,

send an email to [email protected].

Quoted 1 line

To post to this group, send email to

[email protected].

Quoted 1 line

To view this discussion on the web visithttps://groups.google.com/d/msgid/outreachy-kernel/20160228033629.GA7898%4

0sandhya.

Quoted 2 lines

For more options, visit https://groups.google.com/d/optout.

#4 lore raw

Thanks for suggestion !!. I shall try to fix proper return error codes through another patch .

Quoted 101 lines · Julia Lawall
Quoted 1 line · sandhya bankar

The mkimage(..) is returning error i.e return 1 from below error code

path,

Quoted 5 lines

First error condition is if kzalloc(..) has failed,then return 1. Second error condition is if chunk mismatch then it return 1.

I also checked the prism2_fwapply(..) in every error condition,it is returning 1,so i have used return 1 in above patch.

Yes, I see that they are returning 1 everywhere, so there is no problem with the patch. But the whole code should be improved to return proper error codes, like -ENOMEM on allocation failure, rather than 1. There are even some comments in the code saying that typically an error code is returned on failure, but that is not the case. 1 indeed looks more like success (true).

julia

Quoted 5 lines

Do you have any suggestion ?

On Sun, Feb 28, 2016 at 5:31 PM, Julia Lawall <[email protected]>

Quoted 59 lines · wrote:

      On Sun, 28 Feb 2016, Sandhya Bankar wrote:

      > Handle error condition.
      > Below semantic patch found,their is double assignment to
      'result' variable.so instead of deleting double
      assignment,handling error condition for "result = mkimage(..)".
      > @@
      > expression e1,e2,e3;
      > @@
      >
      > (
      >  (<+...e1++...+>)=e2;
      > |
      >  (<+...e1--...+>)=e2;
      > |
      >  (<+...++e1...+>)=e2;
      > |
      >  (<+...--e1...+>)=e2;
      > |
      > e1=e2;
      > e1 = <+...e1...+>;
      > |
      > *e1=e2;
      > *e1=e3;
      >
      > Signed-off-by: Sandhya Bankar <[email protected]>
      > ---
      >  drivers/staging/wlan-ng/prism2fw.c | 4 ++++
      >  1 file changed, 4 insertions(+)
      >
      > diff --git a/drivers/staging/wlan-ng/prism2fw.c
      b/drivers/staging/wlan-ng/prism2fw.c
      > index 8fc80df..35fd5fb 100644
      > --- a/drivers/staging/wlan-ng/prism2fw.c
      > +++ b/drivers/staging/wlan-ng/prism2fw.c
      > @@ -333,6 +333,10 @@ static int prism2_fwapply(const struct
      ihex_binrec *rfptr,
      >
      >       /* Make the image chunks */
      >       result = mkimage(fchunk, &nfchunks);
      > +     if (result) {
      > +             netdev_err(wlandev->netdev, "Failed to make
      image chunk.\n");
      > +             return 1;
      > +     }

Perhaps it would be possible to move this code away from using 1 for failure, towards using actual error codes.

julia

Quoted 6 lines
      /* Do any plugging */
      result = plugimage(fchunk, nfchunks, s3plug, ns3plug, &pda);

--
1.8.3.4

Quoted 8 lines
Quoted 2 lines

--
You received this message because you are subscribed to the Google

Groups "outreachy-kernel" group.

Quoted 1 line

To unsubscribe from this group and stop receiving emails from it,

send an email to [email protected].

Quoted 1 line

To post to this group, send email to

[email protected].

Quoted 1 line

To view this discussion on the web visithttps://

groups.google.com/d/msgid/outreachy-kernel/20160228033629.GA7898%4

Quoted 7 lines

0sandhya.

Quoted 2 lines

For more options, visit https://groups.google.com/d/optout.