* [PATCH] [media] dib0700: Fix uninitialized protocol for NEC repeat codes
@ 2016-10-13 13:51 Geert Uytterhoeven
2016-10-13 21:14 ` Sean Young
0 siblings, 1 reply; 6+ messages in thread
From: Geert Uytterhoeven @ 2016-10-13 13:51 UTC (permalink / raw)
To: Sean Young, Mauro Carvalho Chehab
Cc: linux-media, linux-kernel, Geert Uytterhoeven
drivers/media/usb/dvb-usb/dib0700_core.c: In function ‘dib0700_rc_urb_completion’:
drivers/media/usb/dvb-usb/dib0700_core.c:679: warning: ‘protocol’ may be used uninitialized in this function
When receiving an NEC repeat code, protocol is indeed not initialized.
Set it to RC_TYPE_NECX to fix this.
Fixes: 2ceeca0499d74521 ("[media] rc: split nec protocol into its three variants")
Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
Is RC_TYPE_NECX correct, or should it be RC_TYPE_NEC?
I used RC_TYPE_NECX based on the checks for {,not_}data and
{,not_}system for the other cases.
---
drivers/media/usb/dvb-usb/dib0700_core.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/media/usb/dvb-usb/dib0700_core.c b/drivers/media/usb/dvb-usb/dib0700_core.c
index f3196658fb700706..5878ae4d20ad27ed 100644
--- a/drivers/media/usb/dvb-usb/dib0700_core.c
+++ b/drivers/media/usb/dvb-usb/dib0700_core.c
@@ -718,6 +718,7 @@ static void dib0700_rc_urb_completion(struct urb *purb)
poll_reply->nec.data == 0x00 &&
poll_reply->nec.not_data == 0xff) {
poll_reply->data_state = 2;
+ protocol = RC_TYPE_NECX;
break;
}
--
1.9.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] [media] dib0700: Fix uninitialized protocol for NEC repeat codes
2016-10-13 13:51 [PATCH] [media] dib0700: Fix uninitialized protocol for NEC repeat codes Geert Uytterhoeven
@ 2016-10-13 21:14 ` Sean Young
2016-10-13 21:28 ` [PATCH] [media] dib0700: fix nec repeat handling Sean Young
0 siblings, 1 reply; 6+ messages in thread
From: Sean Young @ 2016-10-13 21:14 UTC (permalink / raw)
To: Geert Uytterhoeven; +Cc: Mauro Carvalho Chehab, linux-media, linux-kernel
On Thu, Oct 13, 2016 at 03:51:39PM +0200, Geert Uytterhoeven wrote:
> drivers/media/usb/dvb-usb/dib0700_core.c: In function ‘dib0700_rc_urb_completion’:
> drivers/media/usb/dvb-usb/dib0700_core.c:679: warning: ‘protocol’ may be used uninitialized in this function
>
> When receiving an NEC repeat code, protocol is indeed not initialized.
> Set it to RC_TYPE_NECX to fix this.
>
> Fixes: 2ceeca0499d74521 ("[media] rc: split nec protocol into its three variants")
> Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
> ---
> Is RC_TYPE_NECX correct, or should it be RC_TYPE_NEC?
> I used RC_TYPE_NECX based on the checks for {,not_}data and
> {,not_}system for the other cases.
It should be the protocol that the last scancode was received with. This
code path is very broken; it calls:
rc_keydown(d->rc_dev, protocol, keycode, toggle);
But keycode in this codepath is never set. Luckily keycode is declared as:
u32 uninitialized_var(keycode);
I've got another patch for this which I'll send as a reply to this.
Sean
> ---
> drivers/media/usb/dvb-usb/dib0700_core.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/media/usb/dvb-usb/dib0700_core.c b/drivers/media/usb/dvb-usb/dib0700_core.c
> index f3196658fb700706..5878ae4d20ad27ed 100644
> --- a/drivers/media/usb/dvb-usb/dib0700_core.c
> +++ b/drivers/media/usb/dvb-usb/dib0700_core.c
> @@ -718,6 +718,7 @@ static void dib0700_rc_urb_completion(struct urb *purb)
> poll_reply->nec.data == 0x00 &&
> poll_reply->nec.not_data == 0xff) {
> poll_reply->data_state = 2;
> + protocol = RC_TYPE_NECX;
> break;
> }
>
> --
> 1.9.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] [media] dib0700: fix nec repeat handling
2016-10-13 21:14 ` Sean Young
@ 2016-10-13 21:28 ` Sean Young
2016-10-29 21:04 ` Arnd Bergmann
0 siblings, 1 reply; 6+ messages in thread
From: Sean Young @ 2016-10-13 21:28 UTC (permalink / raw)
To: Geert Uytterhoeven; +Cc: Mauro Carvalho Chehab, linux-media, linux-kernel
When receiving a nec repeat, ensure the correct scancode is repeated
rather than a random value from the stack. This removes the need
for the bogus uninitialized_var() and also fixes the warnings:
drivers/media/usb/dvb-usb/dib0700_core.c: In function ‘dib0700_rc_urb_completion’:
drivers/media/usb/dvb-usb/dib0700_core.c:679: warning: ‘protocol’ may be used uninitialized in this function
Signed-off-by: Sean Young <sean@mess.org>
---
drivers/media/usb/dvb-usb/dib0700_core.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/media/usb/dvb-usb/dib0700_core.c b/drivers/media/usb/dvb-usb/dib0700_core.c
index f319665..5bb23ef 100644
--- a/drivers/media/usb/dvb-usb/dib0700_core.c
+++ b/drivers/media/usb/dvb-usb/dib0700_core.c
@@ -677,7 +677,7 @@ static void dib0700_rc_urb_completion(struct urb *purb)
struct dvb_usb_device *d = purb->context;
struct dib0700_rc_response *poll_reply;
enum rc_type protocol;
- u32 uninitialized_var(keycode);
+ u32 keycode;
u8 toggle;
deb_info("%s()\n", __func__);
@@ -718,7 +718,8 @@ static void dib0700_rc_urb_completion(struct urb *purb)
poll_reply->nec.data == 0x00 &&
poll_reply->nec.not_data == 0xff) {
poll_reply->data_state = 2;
- break;
+ rc_repeat(d->rc_dev);
+ goto resubmit;
}
if ((poll_reply->nec.data ^ poll_reply->nec.not_data) != 0xff) {
--
2.7.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] [media] dib0700: fix nec repeat handling
2016-10-13 21:28 ` [PATCH] [media] dib0700: fix nec repeat handling Sean Young
@ 2016-10-29 21:04 ` Arnd Bergmann
2016-11-02 16:45 ` Sean Young
0 siblings, 1 reply; 6+ messages in thread
From: Arnd Bergmann @ 2016-10-29 21:04 UTC (permalink / raw)
To: Sean Young, linux-media
Cc: Geert Uytterhoeven, linux-kernel, Mauro Carvalho Chehab
On Thursday, October 13, 2016 10:28:44 PM CEST Sean Young wrote:
> When receiving a nec repeat, ensure the correct scancode is repeated
> rather than a random value from the stack. This removes the need
> for the bogus uninitialized_var() and also fixes the warnings:
>
> drivers/media/usb/dvb-usb/dib0700_core.c: In function ‘dib0700_rc_urb_completion’:
> drivers/media/usb/dvb-usb/dib0700_core.c:679: warning: ‘protocol’ may be used uninitialized in this function
>
> Signed-off-by: Sean Young <sean@mess.org>
> ---
> drivers/media/usb/dvb-usb/dib0700_core.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
Acked-by: Arnd Bergmann <arnd@arndb.de>
Cc: stable@vger.kernel.org
Fixes: 2ceeca0499d7 ("[media] rc: split nec protocol into its three variants")
Fixes: d3c501d1938c ("V4L/DVB: dib0700: Fix RC protocol logic to properly handle NEC/NECx and RC-5")
The warning is gone for me too, so this obsoletes both
https://patchwork.linuxtv.org/patch/37494/ and
https://patchwork.kernel.org/patch/9380747/
Can we get this patch merged into v4.9 soonish? The warning
is currently disabled, but I'd like to make sure it gets turned
on again by default, and we should fix all the actual bugs in
the process.
Arnd
[I replied to Mauro's other address here as mchehab@s-opensource.com
bounced with "Failed to transport message. Message sending failed
since the following recipients were rejected by the server:
mchehab@s-opensource.com (The server responded: Requested action
not taken: mailbox unavailable invalid DNS MX or A/AAAA resource
record)"]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] [media] dib0700: fix nec repeat handling
2016-10-29 21:04 ` Arnd Bergmann
@ 2016-11-02 16:45 ` Sean Young
2016-11-07 13:04 ` Arnd Bergmann
0 siblings, 1 reply; 6+ messages in thread
From: Sean Young @ 2016-11-02 16:45 UTC (permalink / raw)
To: Arnd Bergmann
Cc: linux-media, Geert Uytterhoeven, linux-kernel,
Mauro Carvalho Chehab
On Sat, Oct 29, 2016 at 11:04:32PM +0200, Arnd Bergmann wrote:
> On Thursday, October 13, 2016 10:28:44 PM CEST Sean Young wrote:
> > When receiving a nec repeat, ensure the correct scancode is repeated
> > rather than a random value from the stack. This removes the need
> > for the bogus uninitialized_var() and also fixes the warnings:
> >
> > drivers/media/usb/dvb-usb/dib0700_core.c: In function ‘dib0700_rc_urb_completion’:
> > drivers/media/usb/dvb-usb/dib0700_core.c:679: warning: ‘protocol’ may be used uninitialized in this function
> >
> > Signed-off-by: Sean Young <sean@mess.org>
> > ---
> > drivers/media/usb/dvb-usb/dib0700_core.c | 5 +++--
> > 1 file changed, 3 insertions(+), 2 deletions(-)
>
> Acked-by: Arnd Bergmann <arnd@arndb.de>
> Cc: stable@vger.kernel.org
> Fixes: 2ceeca0499d7 ("[media] rc: split nec protocol into its three variants")
> Fixes: d3c501d1938c ("V4L/DVB: dib0700: Fix RC protocol logic to properly handle NEC/NECx and RC-5")
>
>
> The warning is gone for me too, so this obsoletes both
> https://patchwork.linuxtv.org/patch/37494/ and
> https://patchwork.kernel.org/patch/9380747/
>
> Can we get this patch merged into v4.9 soonish? The warning
> is currently disabled, but I'd like to make sure it gets turned
> on again by default, and we should fix all the actual bugs in
> the process.
So after writing the patch and submitting it, I've bought the hardware on
ebay. Without this patch you get random scancodes on nec repeats, which
the patch indeed fixes.
Tested-by: Sean Young <sean@mess.org>
Note that this has been broken forever, so it is not a regression, so
does it belong in stable?
Sean
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] [media] dib0700: fix nec repeat handling
2016-11-02 16:45 ` Sean Young
@ 2016-11-07 13:04 ` Arnd Bergmann
0 siblings, 0 replies; 6+ messages in thread
From: Arnd Bergmann @ 2016-11-07 13:04 UTC (permalink / raw)
To: Sean Young
Cc: linux-media, Geert Uytterhoeven, linux-kernel,
Mauro Carvalho Chehab
On Wednesday, November 2, 2016 4:45:09 PM CET Sean Young wrote:
> On Sat, Oct 29, 2016 at 11:04:32PM +0200, Arnd Bergmann wrote:
> > On Thursday, October 13, 2016 10:28:44 PM CEST Sean Young wrote:
> > > When receiving a nec repeat, ensure the correct scancode is repeated
> > > rather than a random value from the stack. This removes the need
> > > for the bogus uninitialized_var() and also fixes the warnings:
> > >
> > > drivers/media/usb/dvb-usb/dib0700_core.c: In function ‘dib0700_rc_urb_completion’:
> > > drivers/media/usb/dvb-usb/dib0700_core.c:679: warning: ‘protocol’ may be used uninitialized in this function
> > >
> > > Signed-off-by: Sean Young <sean@mess.org>
> > > ---
> > > drivers/media/usb/dvb-usb/dib0700_core.c | 5 +++--
> > > 1 file changed, 3 insertions(+), 2 deletions(-)
> >
> > Acked-by: Arnd Bergmann <arnd@arndb.de>
> > Cc: stable@vger.kernel.org
> > Fixes: 2ceeca0499d7 ("[media] rc: split nec protocol into its three variants")
> > Fixes: d3c501d1938c ("V4L/DVB: dib0700: Fix RC protocol logic to properly handle NEC/NECx and RC-5")
> >
> >
> > The warning is gone for me too, so this obsoletes both
> > https://patchwork.linuxtv.org/patch/37494/ and
> > https://patchwork.kernel.org/patch/9380747/
> >
> > Can we get this patch merged into v4.9 soonish? The warning
> > is currently disabled, but I'd like to make sure it gets turned
> > on again by default, and we should fix all the actual bugs in
> > the process.
>
> So after writing the patch and submitting it, I've bought the hardware on
> ebay. Without this patch you get random scancodes on nec repeats, which
> the patch indeed fixes.
>
> Tested-by: Sean Young <sean@mess.org>
Awesome, thanks for testing!
> Note that this has been broken forever, so it is not a regression, so
> does it belong in stable?
I think it does, it doesn't have to be a regression to qualify for
stable kernels, and it is clearly a bugfix.
Arnd
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-11-07 13:05 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-10-13 13:51 [PATCH] [media] dib0700: Fix uninitialized protocol for NEC repeat codes Geert Uytterhoeven
2016-10-13 21:14 ` Sean Young
2016-10-13 21:28 ` [PATCH] [media] dib0700: fix nec repeat handling Sean Young
2016-10-29 21:04 ` Arnd Bergmann
2016-11-02 16:45 ` Sean Young
2016-11-07 13:04 ` Arnd Bergmann
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).