| Message ID | 20220228050253.1649-2-tangmeng@uniontech.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v4,1/2] ALSA: core: remove initialise static variables to 0 | expand |
On Mon, 2022-02-28 at 13:02 +0800, Meng Tang wrote: > Return the result from file->f_op->open() directly instead of > taking this in another redundant variable. Make the typical > return the last statement, return early and reduce the indentation > too. > > Signed-off-by: Meng Tang <tangmeng@uniontech.com> > Signed-off-by: Joe Perches <joe@perches.com> Hi Meng Tang. For the next time: it's not necessary (or even good) to add a sign-off for another person unless they specifically authorize one. You wrote and are submitting these changes, I merely gave you simple suggestions as to how you could improve them. cheers, Joe > --- > sound/sound_core.c | 24 ++++++++++++------------ > 1 file changed, 12 insertions(+), 12 deletions(-) > > diff --git a/sound/sound_core.c b/sound/sound_core.c > index aa4a57e488e5..3332fe321737 100644 > --- a/sound/sound_core.c > +++ b/sound/sound_core.c > @@ -577,20 +577,20 @@ static int soundcore_open(struct inode *inode, struct file *file) > new_fops = fops_get(s->unit_fops); > } > spin_unlock(&sound_loader_lock); > - if (new_fops) { > - /* > - * We rely upon the fact that we can't be unloaded while the > - * subdriver is there. > - */ > - int err = 0; > - replace_fops(file, new_fops); > > - if (file->f_op->open) > - err = file->f_op->open(inode,file); > + if (!new_fops) > + return -ENODEV; > > - return err; > - } > - return -ENODEV; > + /* > + * We rely upon the fact that we can't be unloaded while the > + * subdriver is there. > + */ > + replace_fops(file, new_fops); > + > + if (!file->f_op->open) > + return -ENODEV; > + > + return file->f_op->open(inode, file); > } > > MODULE_ALIAS_CHARDEV_MAJOR(SOUND_MAJOR);
On 2022/2/28 13:20, Joe Perches wrote: > Hi Meng Tang. > > For the next time: it's not necessary (or even good) to add a sign-off > for another person unless they specifically authorize one. > > You wrote and are submitting these changes, I merely gave you simple > suggestions as to how you could improve them. > > cheers, Joe > Okay，thanks for the heads up and suggestions. Cheers, Meng
On Mon, 28 Feb 2022 06:20:45 +0100, Joe Perches wrote: > > On Mon, 2022-02-28 at 13:02 +0800, Meng Tang wrote: > > Return the result from file->f_op->open() directly instead of > > taking this in another redundant variable. Make the typical > > return the last statement, return early and reduce the indentation > > too. > > > > Signed-off-by: Meng Tang <tangmeng@uniontech.com> > > Signed-off-by: Joe Perches <joe@perches.com> > > Hi Meng Tang. > > For the next time: it's not necessary (or even good) to add a sign-off > for another person unless they specifically authorize one. > > You wrote and are submitting these changes, I merely gave you simple > suggestions as to how you could improve them. Joe, would you like to drop your S-o-b lines from those two patches? Or shall I keep them? thanks, Takashi
On Mon, 2022-02-28 at 17:02 +0100, Takashi Iwai wrote: > On Mon, 28 Feb 2022 06:20:45 +0100, Joe Perches wrote: > > On Mon, 2022-02-28 at 13:02 +0800, Meng Tang wrote: > > > Return the result from file->f_op->open() directly instead of > > > taking this in another redundant variable. Make the typical > > > return the last statement, return early and reduce the indentation > > > too. > > > > > > Signed-off-by: Meng Tang <tangmeng@uniontech.com> > > > Signed-off-by: Joe Perches <joe@perches.com> > > > > Hi Meng Tang. > > > > For the next time: it's not necessary (or even good) to add a sign-off > > for another person unless they specifically authorize one. > > > > You wrote and are submitting these changes, I merely gave you simple > > suggestions as to how you could improve them. > > Joe, would you like to drop your S-o-b lines from those two patches? > Or shall I keep them? > > thanks, > > Takashi Hi Takashi. Nominally, the sign-off-by chain shows who pushed these changes upstream and I did not and I am not an upstream aggregator. But whatever you choose is OK. It's not really a concern to me. I do think these changes are ok. cheers, Joe
On Mon, 28 Feb 2022 17:39:21 +0100, Joe Perches wrote: > > On Mon, 2022-02-28 at 17:02 +0100, Takashi Iwai wrote: > > On Mon, 28 Feb 2022 06:20:45 +0100, Joe Perches wrote: > > > On Mon, 2022-02-28 at 13:02 +0800, Meng Tang wrote: > > > > Return the result from file->f_op->open() directly instead of > > > > taking this in another redundant variable. Make the typical > > > > return the last statement, return early and reduce the indentation > > > > too. > > > > > > > > Signed-off-by: Meng Tang <tangmeng@uniontech.com> > > > > Signed-off-by: Joe Perches <joe@perches.com> > > > > > > Hi Meng Tang. > > > > > > For the next time: it's not necessary (or even good) to add a sign-off > > > for another person unless they specifically authorize one. > > > > > > You wrote and are submitting these changes, I merely gave you simple > > > suggestions as to how you could improve them. > > > > Joe, would you like to drop your S-o-b lines from those two patches? > > Or shall I keep them? > > > > thanks, > > > > Takashi > > Hi Takashi. > > Nominally, the sign-off-by chain shows who pushed these changes upstream > and I did not and I am not an upstream aggregator. > > But whatever you choose is OK. > It's not really a concern to me. > I do think these changes are ok. OK, I dropped S-o-b lines and applied the patches now. thanks, Takashi
diff --git a/sound/sound_core.c b/sound/sound_core.c index aa4a57e488e5..3332fe321737 100644 --- a/sound/sound_core.c +++ b/sound/sound_core.c @@ -577,20 +577,20 @@ static int soundcore_open(struct inode *inode, struct file *file) new_fops = fops_get(s->unit_fops); } spin_unlock(&sound_loader_lock); - if (new_fops) { - /* - * We rely upon the fact that we can't be unloaded while the - * subdriver is there. - */ - int err = 0; - replace_fops(file, new_fops); - if (file->f_op->open) - err = file->f_op->open(inode,file); + if (!new_fops) + return -ENODEV; - return err; - } - return -ENODEV; + /* + * We rely upon the fact that we can't be unloaded while the + * subdriver is there. + */ + replace_fops(file, new_fops); + + if (!file->f_op->open) + return -ENODEV; + + return file->f_op->open(inode, file); } MODULE_ALIAS_CHARDEV_MAJOR(SOUND_MAJOR);