lkml.org 
[lkml]   [2017]   [May]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [media-dvb-usb-v2] question about value overwrite
Hi Malcolm,

Quoting Malcolm Priestley <tvboxspy@gmail.com>:

> Hi
>
> On 18/05/17 20:09, Gustavo A. R. Silva wrote:
>>
>> Hello everybody,
>>
>> While looking into Coverity ID 1226934 I ran into the following
>> piece of code at drivers/media/usb/dvb-usb-v2/lmedm04.c:205
>>
>> 205static int lme2510_stream_restart(struct dvb_usb_device *d)
>> 206{
>> 207 struct lme2510_state *st = d->priv;
>> 208 u8 all_pids[] = LME_ALL_PIDS;
>> 209 u8 stream_on[] = LME_ST_ON_W;
>> 210 int ret;
>> 211 u8 rbuff[1];
>> 212 if (st->pid_off)
>> 213 ret = lme2510_usb_talk(d, all_pids, sizeof(all_pids),
>> 214 rbuff, sizeof(rbuff));
>> 215 /*Restart Stream Command*/
>> 216 ret = lme2510_usb_talk(d, stream_on, sizeof(stream_on),
>> 217 rbuff, sizeof(rbuff));
>> 218 return ret;
>> 219}
>
> It is a mistake it should have been ORed ad in |= as
> lme2510_usb_talk only returns three states.
>

I see now. The idea is to code something similar to the following
piece of code in the same file:

242
243 ret |= lme2510_usb_talk(d, pid_buff ,
244 sizeof(pid_buff) , rbuf, sizeof(rbuf));
245
246 if (st->stream_on)
247 ret |= lme2510_stream_restart(d);
248
249 return ret;

right?

So in this case, the following patch would properly fix the bug:

index 924adfd..3ab1754 100644
--- a/drivers/media/usb/dvb-usb-v2/lmedm04.c
+++ b/drivers/media/usb/dvb-usb-v2/lmedm04.c
@@ -207,13 +207,14 @@ static int lme2510_stream_restart(struct
dvb_usb_device *d)
struct lme2510_state *st = d->priv;
u8 all_pids[] = LME_ALL_PIDS;
u8 stream_on[] = LME_ST_ON_W;
- int ret;
+ int ret = 0;
u8 rbuff[1];
+
if (st->pid_off)
ret = lme2510_usb_talk(d, all_pids, sizeof(all_pids),
rbuff, sizeof(rbuff));
/*Restart Stream Command*/
- ret = lme2510_usb_talk(d, stream_on, sizeof(stream_on),
+ ret |= lme2510_usb_talk(d, stream_on, sizeof(stream_on),
rbuff, sizeof(rbuff));
return ret;
}
What do you think?

> So if an error is in the running it will be returned to user.
>
> The first of your patches is better and more or less the same, the
> second would break driver, restart is not an else condition.
>

Thank you for the clarification.
--
Gustavo A. R. Silva






\
 
 \ /
  Last update: 2017-05-18 22:38    [W:0.050 / U:0.820 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site