| Message ID | 149365463117.12922.15518669536847504845.stgit@zeus.hardeman.nu (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, May 01, 2017 at 06:03:51PM +0200, David Härdeman wrote: > If an error is generated, nonseekable_open() shouldn't be called. There is no harm in calling nonseekable_open(), so this commit is misleading. Sean > > Signed-off-by: David Härdeman <david@hardeman.nu> > --- > drivers/media/rc/lirc_dev.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/drivers/media/rc/lirc_dev.c b/drivers/media/rc/lirc_dev.c > index 05f600bd6c67..7f13ed479e1c 100644 > --- a/drivers/media/rc/lirc_dev.c > +++ b/drivers/media/rc/lirc_dev.c > @@ -431,7 +431,7 @@ EXPORT_SYMBOL(lirc_unregister_driver); > int lirc_dev_fop_open(struct inode *inode, struct file *file) > { > struct irctl *ir; > - int retval = 0; > + int retval; > > if (iminor(inode) >= MAX_IRCTL_DEVICES) { > pr_err("open result for %d is -ENODEV\n", iminor(inode)); > @@ -475,9 +475,11 @@ int lirc_dev_fop_open(struct inode *inode, struct file *file) > > ir->open++; > > -error: > nonseekable_open(inode, file); > > + return 0; > + > +error: > return retval; > } > EXPORT_SYMBOL(lirc_dev_fop_open);
On Sun, May 21, 2017 at 09:57:13AM +0100, Sean Young wrote: >On Mon, May 01, 2017 at 06:03:51PM +0200, David Härdeman wrote: >> If an error is generated, nonseekable_open() shouldn't be called. > >There is no harm in calling nonseekable_open(), so this commit is >misleading. I'm not sure why you consider it misleading? If there's an error, the logic thing to do is to error out immediately and not do any further work? >> Signed-off-by: David Härdeman <david@hardeman.nu> >> --- >> drivers/media/rc/lirc_dev.c | 6 ++++-- >> 1 file changed, 4 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/media/rc/lirc_dev.c b/drivers/media/rc/lirc_dev.c >> index 05f600bd6c67..7f13ed479e1c 100644 >> --- a/drivers/media/rc/lirc_dev.c >> +++ b/drivers/media/rc/lirc_dev.c >> @@ -431,7 +431,7 @@ EXPORT_SYMBOL(lirc_unregister_driver); >> int lirc_dev_fop_open(struct inode *inode, struct file *file) >> { >> struct irctl *ir; >> - int retval = 0; >> + int retval; >> >> if (iminor(inode) >= MAX_IRCTL_DEVICES) { >> pr_err("open result for %d is -ENODEV\n", iminor(inode)); >> @@ -475,9 +475,11 @@ int lirc_dev_fop_open(struct inode *inode, struct file *file) >> >> ir->open++; >> >> -error: >> nonseekable_open(inode, file); >> >> + return 0; >> + >> +error: >> return retval; >> } >> EXPORT_SYMBOL(lirc_dev_fop_open);
On Sun, May 28, 2017 at 10:23:37AM +0200, David Härdeman wrote: > On Sun, May 21, 2017 at 09:57:13AM +0100, Sean Young wrote: > >On Mon, May 01, 2017 at 06:03:51PM +0200, David Härdeman wrote: > >> If an error is generated, nonseekable_open() shouldn't be called. > > > >There is no harm in calling nonseekable_open(), so this commit is > >misleading. > > I'm not sure why you consider it misleading? If there's an error, the > logic thing to do is to error out immediately and not do any further > work? The commit message says that nonseekable_open() should not be called, suggesting there is a bug which is not the case. Sean
On Sun, May 28, 2017 at 04:04:30PM +0100, Sean Young wrote: >On Sun, May 28, 2017 at 10:23:37AM +0200, David Härdeman wrote: >> On Sun, May 21, 2017 at 09:57:13AM +0100, Sean Young wrote: >> >On Mon, May 01, 2017 at 06:03:51PM +0200, David Härdeman wrote: >> >> If an error is generated, nonseekable_open() shouldn't be called. >> > >> >There is no harm in calling nonseekable_open(), so this commit is >> >misleading. >> >> I'm not sure why you consider it misleading? If there's an error, the >> logic thing to do is to error out immediately and not do any further >> work? > >The commit message says that nonseekable_open() should not be called, >suggesting there is a bug which is not the case. I'll do another version with an updated commit message then :)
diff --git a/drivers/media/rc/lirc_dev.c b/drivers/media/rc/lirc_dev.c index 05f600bd6c67..7f13ed479e1c 100644 --- a/drivers/media/rc/lirc_dev.c +++ b/drivers/media/rc/lirc_dev.c @@ -431,7 +431,7 @@ EXPORT_SYMBOL(lirc_unregister_driver); int lirc_dev_fop_open(struct inode *inode, struct file *file) { struct irctl *ir; - int retval = 0; + int retval; if (iminor(inode) >= MAX_IRCTL_DEVICES) { pr_err("open result for %d is -ENODEV\n", iminor(inode)); @@ -475,9 +475,11 @@ int lirc_dev_fop_open(struct inode *inode, struct file *file) ir->open++; -error: nonseekable_open(inode, file); + return 0; + +error: return retval; } EXPORT_SYMBOL(lirc_dev_fop_open);
If an error is generated, nonseekable_open() shouldn't be called. Signed-off-by: David Härdeman <david@hardeman.nu> --- drivers/media/rc/lirc_dev.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)