| Message ID | 641210ba-571b-43e7-9007-5633a6c7434c@web.de (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | ALSA: i2c: cs8427: Use common code in snd_cs8427_qsubcode_get() | expand |
On Sun, 15 Sep 2024 13:48:51 +0200, Markus Elfring wrote: > > From: Markus Elfring <elfring@users.sourceforge.net> > Date: Sun, 15 Sep 2024 13:38:29 +0200 > > Add two labels so that a bit of exception handling can be better reused > at the end of this function implementation. > > This issue was detected by using the Coccinelle software. > > Signed-off-by: Markus Elfring <elfring@users.sourceforge.net> The double goto jumps aren't nice, either. I guess we can do clean up better with cleanup.h stuff. thanks, Takashi > --- > sound/i2c/cs8427.c | 17 ++++++++++++----- > 1 file changed, 12 insertions(+), 5 deletions(-) > > diff --git a/sound/i2c/cs8427.c b/sound/i2c/cs8427.c > index 29a1a7a0d050..ee650006cac4 100644 > --- a/sound/i2c/cs8427.c > +++ b/sound/i2c/cs8427.c > @@ -397,18 +397,25 @@ static int snd_cs8427_qsubcode_get(struct snd_kcontrol *kcontrol, > if (err != 1) { > dev_err(device->bus->card->dev, > "unable to send register 0x%x byte to CS8427\n", reg); > - snd_i2c_unlock(device->bus); > - return err < 0 ? err : -EIO; > + goto recheck_err; > } > err = snd_i2c_readbytes(device, ucontrol->value.bytes.data, 10); > if (err != 10) { > dev_err(device->bus->card->dev, > "unable to read Q-subcode bytes from CS8427\n"); > - snd_i2c_unlock(device->bus); > - return err < 0 ? err : -EIO; > + goto recheck_err; > } > + > + err = 0; > +unlock: > snd_i2c_unlock(device->bus); > - return 0; > + return err; > + > +recheck_err: > + if (err >= 0) > + err = -EIO; > + > + goto unlock; > } > > static int snd_cs8427_spdif_info(struct snd_kcontrol *kcontrol, > -- > 2.46.0 >
>> Add two labels so that a bit of exception handling can be better reused >> at the end of this function implementation. The implementation of the function “snd_cs8427_qsubcode_get” was identified as another update candidate. https://elixir.bootlin.com/linux/v6.11/source/sound/i2c/cs8427.c#L385 https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/log/sound/i2c/cs8427.c?h=v6.11 > The double goto jumps aren't nice, either. Would you like to care to avoid a bit of duplicate code from repeated return statements? https://wiki.sei.cmu.edu/confluence/display/c/MEM12-C.+Consider+using+a+goto+chain+when+leaving+a+function+on+error+when+using+and+releasing+resources > I guess we can do clean up better with cleanup.h stuff. Scope-based resource management can eventually be applied also for snd_i2c_unlock() calls. https://elixir.bootlin.com/linux/v6.11/source/include/sound/i2c.h#L76 How do you think about to clarify any software extensions accordingly? Regards, Markus
diff --git a/sound/i2c/cs8427.c b/sound/i2c/cs8427.c index 29a1a7a0d050..ee650006cac4 100644 --- a/sound/i2c/cs8427.c +++ b/sound/i2c/cs8427.c @@ -397,18 +397,25 @@ static int snd_cs8427_qsubcode_get(struct snd_kcontrol *kcontrol, if (err != 1) { dev_err(device->bus->card->dev, "unable to send register 0x%x byte to CS8427\n", reg); - snd_i2c_unlock(device->bus); - return err < 0 ? err : -EIO; + goto recheck_err; } err = snd_i2c_readbytes(device, ucontrol->value.bytes.data, 10); if (err != 10) { dev_err(device->bus->card->dev, "unable to read Q-subcode bytes from CS8427\n"); - snd_i2c_unlock(device->bus); - return err < 0 ? err : -EIO; + goto recheck_err; } + + err = 0; +unlock: snd_i2c_unlock(device->bus); - return 0; + return err; + +recheck_err: + if (err >= 0) + err = -EIO; + + goto unlock; } static int snd_cs8427_spdif_info(struct snd_kcontrol *kcontrol,