| Message ID | 1344347451-4467-1-git-send-email-herton.krzesinski@canonical.com |
|---|---|
| State | New |
| Headers | show |
Looking at the complete code it looks good enough to have this pre-stable/upstream. Just the additional commit message after the sob's looks a bit strange... On 07.08.2012 15:50, Herton Ronaldo Krzesinski wrote: > From: Douglas Bagnall <douglas@paradise.net.nz> > > BugLink: http://bugs.launchpad.net/bugs/1015836 > > As pointed out by Ben Hutchings, after commit 720bb6436, the lock was > being taken and not released when an rc_dev has a NULL raw device. > > Signed-off-by: Douglas Bagnall <douglas@paradise.net.nz> > Reported-by: Ben Hutchings <ben@decadent.org.uk> > Signed-off-by: Herton Ronaldo Krzesinski <herton.krzesinski@canonical.com> > --- > drivers/media/rc/rc-main.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > Commit 720bb6436 missed an mutex_unlock on exit, thanks to Ben Hutchings > for pointing that out. Marked as "SAUCE" since it's not upstream yet, > but it should be included upstream. > > diff --git a/drivers/media/rc/rc-main.c b/drivers/media/rc/rc-main.c > index cabc19c..dcd45d0 100644 > --- a/drivers/media/rc/rc-main.c > +++ b/drivers/media/rc/rc-main.c > @@ -778,9 +778,10 @@ static ssize_t show_protocols(struct device *device, > } else if (dev->raw) { > enabled = dev->raw->enabled_protocols; > allowed = ir_raw_get_allowed_protocols(); > - } else > + } else { > + mutex_unlock(&dev->lock); > return -ENODEV; > - > + } > IR_dprintk(1, "allowed - 0x%llx, enabled - 0x%llx\n", > (long long)allowed, > (long long)enabled); >
On 08/07/2012 08:01 AM, Stefan Bader wrote: > Looking at the complete code it looks good enough to have this > pre-stable/upstream. Just the additional commit message after the sob's > looks a bit strange... Everything after the first '---' until a valid "^diff" is elided by 'git am'. > On 07.08.2012 15:50, Herton Ronaldo Krzesinski wrote: >> From: Douglas Bagnall <douglas@paradise.net.nz> >> >> BugLink: http://bugs.launchpad.net/bugs/1015836 >> >> As pointed out by Ben Hutchings, after commit 720bb6436, the lock was >> being taken and not released when an rc_dev has a NULL raw device. >> >> Signed-off-by: Douglas Bagnall <douglas@paradise.net.nz> >> Reported-by: Ben Hutchings <ben@decadent.org.uk> >> Signed-off-by: Herton Ronaldo Krzesinski <herton.krzesinski@canonical.com> >> --- >> drivers/media/rc/rc-main.c | 5 +++-- >> 1 file changed, 3 insertions(+), 2 deletions(-) >> >> Commit 720bb6436 missed an mutex_unlock on exit, thanks to Ben Hutchings >> for pointing that out. Marked as "SAUCE" since it's not upstream yet, >> but it should be included upstream. >> >> diff --git a/drivers/media/rc/rc-main.c b/drivers/media/rc/rc-main.c >> index cabc19c..dcd45d0 100644 >> --- a/drivers/media/rc/rc-main.c >> +++ b/drivers/media/rc/rc-main.c >> @@ -778,9 +778,10 @@ static ssize_t show_protocols(struct device *device, >> } else if (dev->raw) { >> enabled = dev->raw->enabled_protocols; >> allowed = ir_raw_get_allowed_protocols(); >> - } else >> + } else { >> + mutex_unlock(&dev->lock); >> return -ENODEV; >> - >> + } >> IR_dprintk(1, "allowed - 0x%llx, enabled - 0x%llx\n", >> (long long)allowed, >> (long long)enabled); >> > > > >
On Tue, Aug 07, 2012 at 04:01:56PM +0200, Stefan Bader wrote: > Looking at the complete code it looks good enough to have this > pre-stable/upstream. Just the additional commit message after the sob's > looks a bit strange... It was just me commenting about the patch, it's not part of the original changelog of the patch sent upstream, didn't want that included when the patch is applied, so it's below diffstat.
On 07.08.2012 16:08, Herton Ronaldo Krzesinski wrote: > On Tue, Aug 07, 2012 at 04:01:56PM +0200, Stefan Bader wrote: >> Looking at the complete code it looks good enough to have this >> pre-stable/upstream. Just the additional commit message after the sob's >> looks a bit strange... > > It was just me commenting about the patch, it's not part of the original > changelog of the patch sent upstream, didn't want that included when the > patch is applied, so it's below diffstat. > Ah ok, I was not sure it is intended that way.
diff --git a/drivers/media/rc/rc-main.c b/drivers/media/rc/rc-main.c index cabc19c..dcd45d0 100644 --- a/drivers/media/rc/rc-main.c +++ b/drivers/media/rc/rc-main.c @@ -778,9 +778,10 @@ static ssize_t show_protocols(struct device *device, } else if (dev->raw) { enabled = dev->raw->enabled_protocols; allowed = ir_raw_get_allowed_protocols(); - } else + } else { + mutex_unlock(&dev->lock); return -ENODEV; - + } IR_dprintk(1, "allowed - 0x%llx, enabled - 0x%llx\n", (long long)allowed, (long long)enabled);