* [PATCH] staging: emxx_udc: emxx_udc: Removed variables that is never used
@ 2015-01-28 22:42 Rickard Strandqvist
2015-01-29 2:37 ` Chris Rorvick
0 siblings, 1 reply; 8+ messages in thread
From: Rickard Strandqvist @ 2015-01-28 22:42 UTC (permalink / raw)
To: Greg Kroah-Hartman, Vincenzo Scotti
Cc: Rickard Strandqvist, Roberta Dobrescu, Sachin Kamat,
Simon Horman, Ebru Akagunduz, Magnus Damm, devel, linux-kernel
Variable ar assigned a value that is never used.
I have also removed all the code that thereby serves no purpose.
This was found using a static code analysis program called cppcheck
Signed-off-by: Rickard Strandqvist <rickard_strandqvist@spectrumdigital.se>
---
drivers/staging/emxx_udc/emxx_udc.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/staging/emxx_udc/emxx_udc.c b/drivers/staging/emxx_udc/emxx_udc.c
index eb178fc..b916fab 100644
--- a/drivers/staging/emxx_udc/emxx_udc.c
+++ b/drivers/staging/emxx_udc/emxx_udc.c
@@ -2974,10 +2974,10 @@ static int nbu2ss_ep_fifo_status(struct usb_ep *_ep)
spin_lock_irqsave(&udc->lock, flags);
if (ep->epnum == 0) {
- data = _nbu2ss_readl(&preg->EP0_LENGTH) & EP0_LDATA;
+ _nbu2ss_readl(&preg->EP0_LENGTH) & EP0_LDATA;
} else {
- data = _nbu2ss_readl(&preg->EP_REGS[ep->epnum-1].EP_LEN_DCNT)
+ _nbu2ss_readl(&preg->EP_REGS[ep->epnum-1].EP_LEN_DCNT)
& EPn_LDATA;
}
@@ -3264,12 +3264,11 @@ static void __init nbu2ss_drv_set_ep_info(
if (isdigit(name[2])) {
long num;
- int res;
char tempbuf[2];
tempbuf[0] = name[2];
tempbuf[1] = '\0';
- res = kstrtol(tempbuf, 16, &num);
+ kstrtol(tempbuf, 16, &num);
if (num == 0)
ep->ep.maxpacket = EP0_PACKETSIZE;
--
1.7.10.4
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] staging: emxx_udc: emxx_udc: Removed variables that is never used
2015-01-28 22:42 [PATCH] staging: emxx_udc: emxx_udc: Removed variables that is never used Rickard Strandqvist
@ 2015-01-29 2:37 ` Chris Rorvick
2015-01-29 21:52 ` Rickard Strandqvist
0 siblings, 1 reply; 8+ messages in thread
From: Chris Rorvick @ 2015-01-29 2:37 UTC (permalink / raw)
To: Rickard Strandqvist
Cc: Greg Kroah-Hartman, Vincenzo Scotti, Roberta Dobrescu,
Sachin Kamat, Simon Horman, Ebru Akagunduz, Magnus Damm, devel,
linux-kernel
On Wed, Jan 28, 2015 at 4:42 PM, Rickard Strandqvist
<rickard_strandqvist@spectrumdigital.se> wrote:
> Variable ar assigned a value that is never used.
> I have also removed all the code that thereby serves no purpose.
Each of these changes adds a warning ...
> diff --git a/drivers/staging/emxx_udc/emxx_udc.c b/drivers/staging/emxx_udc/emxx_udc.c
> index eb178fc..b916fab 100644
> --- a/drivers/staging/emxx_udc/emxx_udc.c
> +++ b/drivers/staging/emxx_udc/emxx_udc.c
> @@ -2974,10 +2974,10 @@ static int nbu2ss_ep_fifo_status(struct usb_ep *_ep)
> spin_lock_irqsave(&udc->lock, flags);
>
> if (ep->epnum == 0) {
> - data = _nbu2ss_readl(&preg->EP0_LENGTH) & EP0_LDATA;
> + _nbu2ss_readl(&preg->EP0_LENGTH) & EP0_LDATA;
.../linux/drivers/staging/emxx_udc/emxx_udc.c:2977:36: warning: value
computed is not used [-Wunused-value]
_nbu2ss_readl(&preg->EP0_LENGTH) & EP0_LDATA;
^
> } else {
> - data = _nbu2ss_readl(&preg->EP_REGS[ep->epnum-1].EP_LEN_DCNT)
> + _nbu2ss_readl(&preg->EP_REGS[ep->epnum-1].EP_LEN_DCNT)
> & EPn_LDATA;
.../linux/drivers/staging/emxx_udc/emxx_udc.c:2981:4: warning: value
computed is not used [-Wunused-value]
& EPn_LDATA;
^
> }
>
> @@ -3264,12 +3264,11 @@ static void __init nbu2ss_drv_set_ep_info(
> if (isdigit(name[2])) {
>
> long num;
> - int res;
> char tempbuf[2];
>
> tempbuf[0] = name[2];
> tempbuf[1] = '\0';
> - res = kstrtol(tempbuf, 16, &num);
> + kstrtol(tempbuf, 16, &num);
.../linux/drivers/staging/emxx_udc/emxx_udc.c:3271:3: warning:
ignoring return value of ‘kstrtol’, declared with attribute
warn_unused_result [-Wunused-result]
kstrtol(tempbuf, 16, &num);
^
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] staging: emxx_udc: emxx_udc: Removed variables that is never used
2015-01-29 2:37 ` Chris Rorvick
@ 2015-01-29 21:52 ` Rickard Strandqvist
2015-01-29 23:46 ` Chris Rorvick
0 siblings, 1 reply; 8+ messages in thread
From: Rickard Strandqvist @ 2015-01-29 21:52 UTC (permalink / raw)
To: Chris Rorvick
Cc: Greg Kroah-Hartman, Vincenzo Scotti, Roberta Dobrescu,
Sachin Kamat, Simon Horman, Ebru Akagunduz, Magnus Damm, devel,
Linux Kernel Mailing List
2015-01-29 3:37 GMT+01:00 Chris Rorvick <chris@rorvick.com>:
> On Wed, Jan 28, 2015 at 4:42 PM, Rickard Strandqvist
> <rickard_strandqvist@spectrumdigital.se> wrote:
>> Variable ar assigned a value that is never used.
>> I have also removed all the code that thereby serves no purpose.
>
> Each of these changes adds a warning ...
>
>> diff --git a/drivers/staging/emxx_udc/emxx_udc.c b/drivers/staging/emxx_udc/emxx_udc.c
>> index eb178fc..b916fab 100644
>> --- a/drivers/staging/emxx_udc/emxx_udc.c
>> +++ b/drivers/staging/emxx_udc/emxx_udc.c
>> @@ -2974,10 +2974,10 @@ static int nbu2ss_ep_fifo_status(struct usb_ep *_ep)
>> spin_lock_irqsave(&udc->lock, flags);
>>
>> if (ep->epnum == 0) {
>> - data = _nbu2ss_readl(&preg->EP0_LENGTH) & EP0_LDATA;
>> + _nbu2ss_readl(&preg->EP0_LENGTH) & EP0_LDATA;
>
> .../linux/drivers/staging/emxx_udc/emxx_udc.c:2977:36: warning: value
> computed is not used [-Wunused-value]
> _nbu2ss_readl(&preg->EP0_LENGTH) & EP0_LDATA;
> ^
>
>> } else {
>> - data = _nbu2ss_readl(&preg->EP_REGS[ep->epnum-1].EP_LEN_DCNT)
>> + _nbu2ss_readl(&preg->EP_REGS[ep->epnum-1].EP_LEN_DCNT)
>> & EPn_LDATA;
>
> .../linux/drivers/staging/emxx_udc/emxx_udc.c:2981:4: warning: value
> computed is not used [-Wunused-value]
> & EPn_LDATA;
> ^
>
>> }
>>
>> @@ -3264,12 +3264,11 @@ static void __init nbu2ss_drv_set_ep_info(
>> if (isdigit(name[2])) {
>>
>> long num;
>> - int res;
>> char tempbuf[2];
>>
>> tempbuf[0] = name[2];
>> tempbuf[1] = '\0';
>> - res = kstrtol(tempbuf, 16, &num);
>> + kstrtol(tempbuf, 16, &num);
>
> .../linux/drivers/staging/emxx_udc/emxx_udc.c:3271:3: warning:
> ignoring return value of ‘kstrtol’, declared with attribute
> warn_unused_result [-Wunused-result]
> kstrtol(tempbuf, 16, &num);
> ^
Hi
The first two were my fault, stupid of me to let the "& number" remain :(
The last one was more interesting, se below.
But I can not really see how any error should be handled here?
Proposal to change to:
if (kstrtol(tempbuf, 16, &num) == 0 && num == 0)
static inline int __must_check kstrtol(const char *s, unsigned int
base, long *res);
"The __must_check annotation makes use of the gcc warn_unused_result
attribute; it first found its way into the mainline in 2.6.8. If a
function is marked __must_check, the compiler will issue a strong
warning whenever the function is called and its return code is unused.
"
Kind regards
Rickard Strandqvist
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] staging: emxx_udc: emxx_udc: Removed variables that is never used
2015-01-29 21:52 ` Rickard Strandqvist
@ 2015-01-29 23:46 ` Chris Rorvick
2015-01-30 1:58 ` Chris Rorvick
0 siblings, 1 reply; 8+ messages in thread
From: Chris Rorvick @ 2015-01-29 23:46 UTC (permalink / raw)
To: Rickard Strandqvist
Cc: Greg Kroah-Hartman, Vincenzo Scotti, Roberta Dobrescu,
Sachin Kamat, Simon Horman, Ebru Akagunduz, Magnus Damm, devel,
Linux Kernel Mailing List
On Thu, Jan 29, 2015 at 3:52 PM, Rickard Strandqvist
<rickard_strandqvist@spectrumdigital.se> wrote:
> The last one was more interesting, se below.
> But I can not really see how any error should be handled here?
> Proposal to change to:
>
> if (kstrtol(tempbuf, 16, &num) == 0 && num == 0)
That whole chunk of code looks odd. Why the base 16 conversion when
we already know it's a decimal digit? Seems like this would work
without the hassle of the string conversion:
-- >8 --
--- a/drivers/staging/emxx_udc/emxx_udc.c
+++ b/drivers/staging/emxx_udc/emxx_udc.c
@@ -3262,16 +3262,7 @@ static void __init nbu2ss_drv_set_ep_info(
ep->ep.ops = &nbu2ss_ep_ops;
if (isdigit(name[2])) {
-
- long num;
- int res;
- char tempbuf[2];
-
- tempbuf[0] = name[2];
- tempbuf[1] = '\0';
- res = kstrtol(tempbuf, 16, &num);
-
- if (num == 0)
+ if (name[2] == '0')
ep->ep.maxpacket = EP0_PACKETSIZE;
else
ep->ep.maxpacket = EP_PACKETSIZE;
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] staging: emxx_udc: emxx_udc: Removed variables that is never used
2015-01-29 23:46 ` Chris Rorvick
@ 2015-01-30 1:58 ` Chris Rorvick
2015-01-30 14:20 ` Dan Carpenter
0 siblings, 1 reply; 8+ messages in thread
From: Chris Rorvick @ 2015-01-30 1:58 UTC (permalink / raw)
To: Rickard Strandqvist
Cc: Greg Kroah-Hartman, Vincenzo Scotti, Roberta Dobrescu,
Sachin Kamat, Simon Horman, Ebru Akagunduz, Magnus Damm, devel,
Linux Kernel Mailing List
On Thu, Jan 29, 2015 at 5:46 PM, Chris Rorvick <chris@rorvick.com> wrote:
> That whole chunk of code looks odd. Why the base 16 conversion when
> we already know it's a decimal digit? Seems like this would work
> without the hassle of the string conversion:
Hmm, or probably even better to do this where the ep0 check is less contorted.
-- >8 --
diff --git a/drivers/staging/emxx_udc/emxx_udc.c
b/drivers/staging/emxx_udc/emxx_udc.c
index eb178fc..98a1ace 100644
--- a/drivers/staging/emxx_udc/emxx_udc.c
+++ b/drivers/staging/emxx_udc/emxx_udc.c
@@ -3261,25 +3261,6 @@ static void __init nbu2ss_drv_set_ep_info(
ep->ep.name = name;
ep->ep.ops = &nbu2ss_ep_ops;
- if (isdigit(name[2])) {
-
- long num;
- int res;
- char tempbuf[2];
-
- tempbuf[0] = name[2];
- tempbuf[1] = '\0';
- res = kstrtol(tempbuf, 16, &num);
-
- if (num == 0)
- ep->ep.maxpacket = EP0_PACKETSIZE;
- else
- ep->ep.maxpacket = EP_PACKETSIZE;
-
- } else {
- ep->ep.maxpacket = EP_PACKETSIZE;
- }
-
list_add_tail(&ep->ep.ep_list, &udc->gadget.ep_list);
INIT_LIST_HEAD(&ep->queue);
}
@@ -3293,8 +3274,12 @@ static void __init nbu2ss_drv_ep_init(struct
nbu2ss_udc *udc)
udc->gadget.ep0 = &udc->ep[0].ep;
- for (i = 0; i < NUM_ENDPOINTS; i++)
- nbu2ss_drv_set_ep_info(udc, &udc->ep[i], gp_ep_name[i]);
+ for (i = 0; i < NUM_ENDPOINTS; i++) {
+ struct nbu2ss_ep *ep = &udc->ep[i];
+
+ nbu2ss_drv_set_ep_info(udc, ep, gp_ep_name[i]);
+ ep->ep.maxpacket = i == 0 ? EP0_PACKETSIZE : EP_PACKETSIZE;
+ }
list_del_init(&udc->ep[0].ep.ep_list);
}
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] staging: emxx_udc: emxx_udc: Removed variables that is never used
2015-01-30 1:58 ` Chris Rorvick
@ 2015-01-30 14:20 ` Dan Carpenter
2015-01-30 17:35 ` Rickard Strandqvist
0 siblings, 1 reply; 8+ messages in thread
From: Dan Carpenter @ 2015-01-30 14:20 UTC (permalink / raw)
To: Chris Rorvick
Cc: Rickard Strandqvist, devel, Greg Kroah-Hartman, Magnus Damm,
Sachin Kamat, Vincenzo Scotti, Roberta Dobrescu, Simon Horman,
Linux Kernel Mailing List
Yes. Please send that as a patch which can be applied.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] staging: emxx_udc: emxx_udc: Removed variables that is never used
2015-01-30 14:20 ` Dan Carpenter
@ 2015-01-30 17:35 ` Rickard Strandqvist
2015-01-30 23:21 ` Chris Rorvick
0 siblings, 1 reply; 8+ messages in thread
From: Rickard Strandqvist @ 2015-01-30 17:35 UTC (permalink / raw)
To: Dan Carpenter
Cc: Chris Rorvick, devel, Greg Kroah-Hartman, Magnus Damm,
Sachin Kamat, Vincenzo Scotti, Roberta Dobrescu, Simon Horman,
Linux Kernel Mailing List
2015-01-30 15:20 GMT+01:00 Dan Carpenter <dan.carpenter@oracle.com>:
> Yes. Please send that as a patch which can be applied.
>
> regards,
> dan carpenter
>
Hi
Okay, I'll do it this weekend.
Or do you do it Chris?
Kind regards
Rickard Strandqvist
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] staging: emxx_udc: emxx_udc: Removed variables that is never used
2015-01-30 17:35 ` Rickard Strandqvist
@ 2015-01-30 23:21 ` Chris Rorvick
0 siblings, 0 replies; 8+ messages in thread
From: Chris Rorvick @ 2015-01-30 23:21 UTC (permalink / raw)
To: Rickard Strandqvist
Cc: Dan Carpenter, devel, Greg Kroah-Hartman, Magnus Damm,
Sachin Kamat, Vincenzo Scotti, Roberta Dobrescu, Simon Horman,
Linux Kernel Mailing List
On Fri, Jan 30, 2015 at 11:35 AM, Rickard Strandqvist
<rickard_strandqvist@spectrumdigital.se> wrote:
> 2015-01-30 15:20 GMT+01:00 Dan Carpenter <dan.carpenter@oracle.com>:
>> Yes. Please send that as a patch which can be applied.
>>
>> regards,
>> dan carpenter
>>
>
>
> Hi
>
> Okay, I'll do it this weekend.
> Or do you do it Chris?
I'll send out a patch removing nbu2ss_drv_set_dp_info() in a minute.
This only inflates the init function to a few more than 20 lines while
making the code far easier to understand.
Regards,
Chris
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2015-01-30 23:21 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-01-28 22:42 [PATCH] staging: emxx_udc: emxx_udc: Removed variables that is never used Rickard Strandqvist
2015-01-29 2:37 ` Chris Rorvick
2015-01-29 21:52 ` Rickard Strandqvist
2015-01-29 23:46 ` Chris Rorvick
2015-01-30 1:58 ` Chris Rorvick
2015-01-30 14:20 ` Dan Carpenter
2015-01-30 17:35 ` Rickard Strandqvist
2015-01-30 23:21 ` Chris Rorvick
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.