[alsa-devel] [PATCH 0/13] ALSA: Deletion of some unnecessary checks
From: Markus Elfring elfring@users.sourceforge.net Date: Sun, 4 Jan 2015 11:50:12 +0100
Further update suggestions were taken into account after several patches were applied from static source code analysis.
Markus Elfring (3): seq: Deletion of unnecessary checks before the function call "snd_midi_event_free" oss: Deletion of unnecessary checks before the function call "vfree" emu10k1: Delete an unnecessary check before the function call "kfree" oxygen: Delete an unnecessary check before the function call "snd_pcm_suspend" emux: Delete an unnecessary check before the function call "snd_sf_free" ASoC: Intel: Delete an unnecessary check before the function call "sst_dma_free" ASoC: fsi: Deletion of unnecessary checks before the function call "clk_enable" ASoC: Intel: Delete an unnecessary check before the function call "release_firmware" i2sbus: Delete an unnecessary check before the function call "snd_pcm_suspend_all" sb: Delete an unnecessary check before the function call "snd_emux_free" Deletion of checks before the function call "iounmap" msnd: One function call less in snd_msnd_attach() after error detection msnd: Fix centralized exiting from snd_msnd_attach()
sound/aoa/soundbus/i2sbus/core.c | 13 ++++++------- sound/arm/aaci.c | 4 ++-- sound/drivers/ml403-ac97cr.c | 3 +-- sound/isa/msnd/msnd_pinnacle.c | 31 ++++++++++++++++--------------- sound/parisc/harmony.c | 4 +--- sound/pci/ad1889.c | 5 +---- sound/pci/asihpi/hpioctl.c | 6 ++---- sound/pci/atiixp.c | 3 +-- sound/pci/atiixp_modem.c | 3 +-- sound/pci/aw2/aw2-alsa.c | 4 +--- sound/pci/bt87x.c | 3 +-- sound/pci/cs4281.c | 6 ++---- sound/pci/cs46xx/cs46xx_lib.c | 4 ++-- sound/pci/ctxfi/cthw20k1.c | 5 +---- sound/pci/ctxfi/cthw20k2.c | 5 +---- sound/pci/echoaudio/echoaudio.c | 6 +----- sound/pci/hda/hda_intel.c | 3 +-- sound/pci/lola/lola.c | 6 ++---- sound/pci/mixart/mixart.c | 7 +++---- sound/pci/nm256/nm256.c | 6 ++---- sound/pci/rme9652/hdsp.c | 4 +--- sound/pci/rme9652/hdspm.c | 4 +--- sound/pci/rme9652/rme9652.c | 3 +-- sound/pci/sis7019.c | 5 +---- sound/pci/ymfpci/ymfpci_main.c | 3 +-- sound/ppc/pmac.c | 15 +++++---------- 26 files changed, 58 insertions(+), 103 deletions(-)
From: Markus Elfring elfring@users.sourceforge.net Date: Sat, 3 Jan 2015 14:54:56 +0100
The snd_midi_event_free() function tests whether its argument is NULL and then returns immediately. Thus the test around the call is not needed.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring elfring@users.sourceforge.net --- sound/core/seq/oss/seq_oss_midi.c | 6 ++---- sound/core/seq/seq_midi.c | 3 +-- 2 files changed, 3 insertions(+), 6 deletions(-)
diff --git a/sound/core/seq/oss/seq_oss_midi.c b/sound/core/seq/oss/seq_oss_midi.c index 3a45696..e79cc44 100644 --- a/sound/core/seq/oss/seq_oss_midi.c +++ b/sound/core/seq/oss/seq_oss_midi.c @@ -237,8 +237,7 @@ snd_seq_oss_midi_check_exit_port(int client, int port) spin_unlock_irqrestore(®ister_lock, flags); snd_use_lock_free(&mdev->use_lock); snd_use_lock_sync(&mdev->use_lock); - if (mdev->coder) - snd_midi_event_free(mdev->coder); + snd_midi_event_free(mdev->coder); kfree(mdev); } spin_lock_irqsave(®ister_lock, flags); @@ -265,8 +264,7 @@ snd_seq_oss_midi_clear_all(void) spin_lock_irqsave(®ister_lock, flags); for (i = 0; i < max_midi_devs; i++) { if ((mdev = midi_devs[i]) != NULL) { - if (mdev->coder) - snd_midi_event_free(mdev->coder); + snd_midi_event_free(mdev->coder); kfree(mdev); midi_devs[i] = NULL; } diff --git a/sound/core/seq/seq_midi.c b/sound/core/seq/seq_midi.c index a1fd77a..68fec77 100644 --- a/sound/core/seq/seq_midi.c +++ b/sound/core/seq/seq_midi.c @@ -268,8 +268,7 @@ static void snd_seq_midisynth_delete(struct seq_midisynth *msynth) snd_seq_event_port_detach(msynth->seq_client, msynth->seq_port); }
- if (msynth->parser) - snd_midi_event_free(msynth->parser); + snd_midi_event_free(msynth->parser); }
/* register new midi synth port */
From: Markus Elfring elfring@users.sourceforge.net Date: Sat, 3 Jan 2015 15:10:52 +0100
The vfree() function performs also input parameter validation. Thus the test around the call is not needed.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring elfring@users.sourceforge.net --- sound/oss/pss.c | 2 +- sound/oss/trix.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/sound/oss/pss.c b/sound/oss/pss.c index ca0d6e9..81314f9 100644 --- a/sound/oss/pss.c +++ b/sound/oss/pss.c @@ -1228,7 +1228,7 @@ static void __exit cleanup_pss(void) { if(!pss_no_sound) { - if(fw_load && pss_synth) + if (fw_load) vfree(pss_synth); if(pssmss) unload_pss_mss(&cfg2); diff --git a/sound/oss/trix.c b/sound/oss/trix.c index 944e0c0..3c494dc 100644 --- a/sound/oss/trix.c +++ b/sound/oss/trix.c @@ -487,7 +487,7 @@ static int __init init_trix(void)
static void __exit cleanup_trix(void) { - if (fw_load && trix_boot) + if (fw_load) vfree(trix_boot); if (sb) unload_trix_sb(&cfg2);
From: Markus Elfring elfring@users.sourceforge.net Date: Sat, 3 Jan 2015 17:06:04 +0100
The kfree() function tests whether its argument is NULL and then returns immediately. Thus the test around the call is not needed.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring elfring@users.sourceforge.net --- sound/pci/emu10k1/p16v.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/sound/pci/emu10k1/p16v.c b/sound/pci/emu10k1/p16v.c index b672755..3c60b43 100644 --- a/sound/pci/emu10k1/p16v.c +++ b/sound/pci/emu10k1/p16v.c @@ -166,11 +166,8 @@ static struct snd_pcm_hardware snd_p16v_capture_hw = { static void snd_p16v_pcm_free_substream(struct snd_pcm_runtime *runtime) { struct snd_emu10k1_pcm *epcm = runtime->private_data; - - if (epcm) { - /* dev_dbg(emu->card->dev, "epcm free: %p\n", epcm); */ - kfree(epcm); - } + + kfree(epcm); }
/* open_playback callback */
From: Markus Elfring elfring@users.sourceforge.net Date: Sat, 3 Jan 2015 17:37:28 +0100
The snd_pcm_suspend() function tests whether its argument is NULL and then returns immediately. Thus the test around the call is not needed.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring elfring@users.sourceforge.net --- sound/pci/oxygen/oxygen_lib.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/sound/pci/oxygen/oxygen_lib.c b/sound/pci/oxygen/oxygen_lib.c index b67e306..61a62c0 100644 --- a/sound/pci/oxygen/oxygen_lib.c +++ b/sound/pci/oxygen/oxygen_lib.c @@ -736,8 +736,7 @@ static int oxygen_pci_suspend(struct device *dev) snd_power_change_state(card, SNDRV_CTL_POWER_D3hot);
for (i = 0; i < PCM_COUNT; ++i) - if (chip->streams[i]) - snd_pcm_suspend(chip->streams[i]); + snd_pcm_suspend(chip->streams[i]);
if (chip->model.suspend) chip->model.suspend(chip);
From: Markus Elfring elfring@users.sourceforge.net Date: Sat, 3 Jan 2015 18:28:21 +0100
The snd_sf_free() function tests whether its argument is NULL and then returns immediately. Thus the test around the call is not needed.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring elfring@users.sourceforge.net --- sound/synth/emux/emux.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/sound/synth/emux/emux.c b/sound/synth/emux/emux.c index 9352207..f27a1c8 100644 --- a/sound/synth/emux/emux.c +++ b/sound/synth/emux/emux.c @@ -160,12 +160,8 @@ int snd_emux_free(struct snd_emux *emu) snd_emux_detach_seq_oss(emu); #endif snd_emux_detach_seq(emu); - snd_emux_delete_hwdep(emu); - - if (emu->sflist) - snd_sf_free(emu->sflist); - + snd_sf_free(emu->sflist); kfree(emu->voices); kfree(emu->name); kfree(emu);
From: Markus Elfring elfring@users.sourceforge.net Date: Sat, 3 Jan 2015 19:03:55 +0100
The sst_dma_free() function tests whether its argument is NULL and then returns immediately. Thus the test around the call is not needed.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring elfring@users.sourceforge.net --- sound/soc/intel/sst-dsp.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/sound/soc/intel/sst-dsp.c b/sound/soc/intel/sst-dsp.c index 86e4108..64e9421 100644 --- a/sound/soc/intel/sst-dsp.c +++ b/sound/soc/intel/sst-dsp.c @@ -410,8 +410,7 @@ void sst_dsp_free(struct sst_dsp *sst) if (sst->ops->free) sst->ops->free(sst);
- if (sst->dma) - sst_dma_free(sst->dma); + sst_dma_free(sst->dma); } EXPORT_SYMBOL_GPL(sst_dsp_free);
On Sun, Jan 04, 2015 at 02:08:42PM +0100, SF Markus Elfring wrote:
From: Markus Elfring elfring@users.sourceforge.net Date: Sat, 3 Jan 2015 19:03:55 +0100
The sst_dma_free() function tests whether its argument is NULL and then returns immediately. Thus the test around the call is not needed.
I'm missing aptches 1-5, are there any dependencies?
At Mon, 5 Jan 2015 14:35:51 +0000, Mark Brown wrote:
On Sun, Jan 04, 2015 at 02:08:42PM +0100, SF Markus Elfring wrote:
From: Markus Elfring elfring@users.sourceforge.net Date: Sat, 3 Jan 2015 19:03:55 +0100
The sst_dma_free() function tests whether its argument is NULL and then returns immediately. Thus the test around the call is not needed.
I'm missing aptches 1-5, are there any dependencies?
No, all individual fixes.
Takashi
On Mon, Jan 05, 2015 at 04:18:07PM +0100, Takashi Iwai wrote:
Mark Brown wrote:
I'm missing aptches 1-5, are there any dependencies?
No, all individual fixes.
OK. SF, in future please always send at least the cover letter to everyone getting patches in the series so they know what's going on.
On Sun, Jan 04, 2015 at 02:08:42PM +0100, SF Markus Elfring wrote:
From: Markus Elfring elfring@users.sourceforge.net Date: Sat, 3 Jan 2015 19:03:55 +0100
The sst_dma_free() function tests whether its argument is NULL and then returns immediately. Thus the test around the call is not needed.
Applied, thanks.
From: Markus Elfring elfring@users.sourceforge.net Date: Sat, 3 Jan 2015 19:25:55 +0100
The clk_enable() function tests whether its argument is NULL and then returns immediately. Thus the test around the call is not needed.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring elfring@users.sourceforge.net --- sound/soc/sh/fsi.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/sound/soc/sh/fsi.c b/sound/soc/sh/fsi.c index 8869971..d49f25f 100644 --- a/sound/soc/sh/fsi.c +++ b/sound/soc/sh/fsi.c @@ -820,12 +820,9 @@ static int fsi_clk_enable(struct device *dev, return ret; }
- if (clock->xck) - clk_enable(clock->xck); - if (clock->ick) - clk_enable(clock->ick); - if (clock->div) - clk_enable(clock->div); + clk_enable(clock->xck); + clk_enable(clock->ick); + clk_enable(clock->div);
clock->count++; }
On Sun, Jan 04, 2015 at 02:11:58PM +0100, SF Markus Elfring wrote:
From: Markus Elfring elfring@users.sourceforge.net Date: Sat, 3 Jan 2015 19:25:55 +0100
The clk_enable() function tests whether its argument is NULL and then returns immediately. Thus the test around the call is not needed.
Applied, thanks.
From: Markus Elfring elfring@users.sourceforge.net Date: Sat, 3 Jan 2015 19:49:37 +0100
The release_firmware() function tests whether its argument is NULL and then returns immediately. Thus the test around the call is not needed.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring elfring@users.sourceforge.net --- sound/soc/intel/sst/sst_loader.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/sound/soc/intel/sst/sst_loader.c b/sound/soc/intel/sst/sst_loader.c index b580f96..7888cd7 100644 --- a/sound/soc/intel/sst/sst_loader.c +++ b/sound/soc/intel/sst/sst_loader.c @@ -324,8 +324,7 @@ void sst_firmware_load_cb(const struct firmware *fw, void *context)
if (ctx->sst_state != SST_RESET || ctx->fw_in_mem != NULL) { - if (fw != NULL) - release_firmware(fw); + release_firmware(fw); mutex_unlock(&ctx->sst_lock); return; }
On Sun, Jan 04, 2015 at 02:14:37PM +0100, SF Markus Elfring wrote:
From: Markus Elfring elfring@users.sourceforge.net Date: Sat, 3 Jan 2015 19:49:37 +0100
The release_firmware() function tests whether its argument is NULL and then returns immediately. Thus the test around the call is not needed.
Applied, thanks.
From: Markus Elfring elfring@users.sourceforge.net Date: Sat, 3 Jan 2015 20:43:01 +0100
The snd_pcm_suspend_all() function tests whether its argument is NULL and then returns immediately. Thus the test around the call is not needed.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring elfring@users.sourceforge.net --- sound/aoa/soundbus/i2sbus/core.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/sound/aoa/soundbus/i2sbus/core.c b/sound/aoa/soundbus/i2sbus/core.c index 4e2b4fb..837ba99 100644 --- a/sound/aoa/soundbus/i2sbus/core.c +++ b/sound/aoa/soundbus/i2sbus/core.c @@ -381,10 +381,8 @@ static int i2sbus_suspend(struct macio_dev* dev, pm_message_t state)
list_for_each_entry(i2sdev, &control->list, item) { /* Notify Alsa */ - if (i2sdev->sound.pcm) { - /* Suspend PCM streams */ - snd_pcm_suspend_all(i2sdev->sound.pcm); - } + /* Suspend PCM streams */ + snd_pcm_suspend_all(i2sdev->sound.pcm);
/* Notify codecs */ list_for_each_entry(cii, &i2sdev->sound.codec_list, list) {
From: Markus Elfring elfring@users.sourceforge.net Date: Sat, 3 Jan 2015 21:02:32 +0100
The snd_emux_free() function tests whether its argument is NULL and then returns immediately. Thus the test around the call is not needed.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring elfring@users.sourceforge.net --- sound/isa/sb/emu8000_synth.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/sound/isa/sb/emu8000_synth.c b/sound/isa/sb/emu8000_synth.c index 95b39be..72332df 100644 --- a/sound/isa/sb/emu8000_synth.c +++ b/sound/isa/sb/emu8000_synth.c @@ -103,8 +103,7 @@ static int snd_emu8000_delete_device(struct snd_seq_device *dev) hw = dev->driver_data; if (hw->pcm) snd_device_free(dev->card, hw->pcm); - if (hw->emu) - snd_emux_free(hw->emu); + snd_emux_free(hw->emu); snd_util_memhdr_free(hw->memhdr); hw->emu = NULL; hw->memhdr = NULL;
From: Markus Elfring elfring@users.sourceforge.net Date: Sat, 3 Jan 2015 22:55:54 +0100
The iounmap() function performs also input parameter validation. Thus the test around the call is not needed.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring elfring@users.sourceforge.net --- sound/aoa/soundbus/i2sbus/core.c | 13 ++++++------- sound/arm/aaci.c | 4 ++-- sound/drivers/ml403-ac97cr.c | 3 +-- sound/isa/msnd/msnd_pinnacle.c | 3 +-- sound/parisc/harmony.c | 4 +--- sound/pci/ad1889.c | 5 +---- sound/pci/asihpi/hpioctl.c | 6 ++---- sound/pci/atiixp.c | 3 +-- sound/pci/atiixp_modem.c | 3 +-- sound/pci/aw2/aw2-alsa.c | 4 +--- sound/pci/bt87x.c | 3 +-- sound/pci/cs4281.c | 6 ++---- sound/pci/cs46xx/cs46xx_lib.c | 4 ++-- sound/pci/ctxfi/cthw20k1.c | 5 +---- sound/pci/ctxfi/cthw20k2.c | 5 +---- sound/pci/echoaudio/echoaudio.c | 6 +----- sound/pci/hda/hda_intel.c | 3 +-- sound/pci/lola/lola.c | 6 ++---- sound/pci/mixart/mixart.c | 7 +++---- sound/pci/nm256/nm256.c | 6 ++---- sound/pci/rme9652/hdsp.c | 4 +--- sound/pci/rme9652/hdspm.c | 4 +--- sound/pci/rme9652/rme9652.c | 3 +-- sound/pci/sis7019.c | 5 +---- sound/pci/ymfpci/ymfpci_main.c | 3 +-- sound/ppc/pmac.c | 15 +++++---------- 26 files changed, 43 insertions(+), 90 deletions(-)
diff --git a/sound/aoa/soundbus/i2sbus/core.c b/sound/aoa/soundbus/i2sbus/core.c index 4e2b4fb..7835045 100644 --- a/sound/aoa/soundbus/i2sbus/core.c +++ b/sound/aoa/soundbus/i2sbus/core.c @@ -74,10 +74,9 @@ static void i2sbus_release_dev(struct device *dev) int i;
i2sdev = container_of(dev, struct i2sbus_dev, sound.ofdev.dev); - - if (i2sdev->intfregs) iounmap(i2sdev->intfregs); - if (i2sdev->out.dbdma) iounmap(i2sdev->out.dbdma); - if (i2sdev->in.dbdma) iounmap(i2sdev->in.dbdma); + iounmap(i2sdev->intfregs); + iounmap(i2sdev->out.dbdma); + iounmap(i2sdev->in.dbdma); for (i = aoa_resource_i2smmio; i <= aoa_resource_rxdbdma; i++) release_and_free_resource(i2sdev->allocated_resource[i]); free_dbdma_descriptor_ring(i2sdev, &i2sdev->out.dbdma_ring); @@ -318,9 +317,9 @@ static int i2sbus_add_dev(struct macio_dev *macio, free_irq(dev->interrupts[i], dev); free_dbdma_descriptor_ring(dev, &dev->out.dbdma_ring); free_dbdma_descriptor_ring(dev, &dev->in.dbdma_ring); - if (dev->intfregs) iounmap(dev->intfregs); - if (dev->out.dbdma) iounmap(dev->out.dbdma); - if (dev->in.dbdma) iounmap(dev->in.dbdma); + iounmap(dev->intfregs); + iounmap(dev->out.dbdma); + iounmap(dev->in.dbdma); for (i=0;i<3;i++) release_and_free_resource(dev->allocated_resource[i]); mutex_destroy(&dev->lock); diff --git a/sound/arm/aaci.c b/sound/arm/aaci.c index 0e83a73..4140b1b 100644 --- a/sound/arm/aaci.c +++ b/sound/arm/aaci.c @@ -889,8 +889,8 @@ static int aaci_probe_ac97(struct aaci *aaci) static void aaci_free_card(struct snd_card *card) { struct aaci *aaci = card->private_data; - if (aaci->base) - iounmap(aaci->base); + + iounmap(aaci->base); }
static struct aaci *aaci_init_card(struct amba_device *dev) diff --git a/sound/drivers/ml403-ac97cr.c b/sound/drivers/ml403-ac97cr.c index ec01de1..bdcb572 100644 --- a/sound/drivers/ml403-ac97cr.c +++ b/sound/drivers/ml403-ac97cr.c @@ -1094,8 +1094,7 @@ static int snd_ml403_ac97cr_free(struct snd_ml403_ac97cr *ml403_ac97cr) if (ml403_ac97cr->capture_irq >= 0) free_irq(ml403_ac97cr->capture_irq, ml403_ac97cr); /* give back "port" */ - if (ml403_ac97cr->port != NULL) - iounmap(ml403_ac97cr->port); + iounmap(ml403_ac97cr->port); kfree(ml403_ac97cr); PDEBUG(INIT_INFO, "free(): (done)\n"); return 0; diff --git a/sound/isa/msnd/msnd_pinnacle.c b/sound/isa/msnd/msnd_pinnacle.c index 65b3682..4c07266 100644 --- a/sound/isa/msnd/msnd_pinnacle.c +++ b/sound/isa/msnd/msnd_pinnacle.c @@ -627,8 +627,7 @@ static int snd_msnd_attach(struct snd_card *card) return 0;
err_release_region: - if (chip->mappedbase) - iounmap(chip->mappedbase); + iounmap(chip->mappedbase); release_mem_region(chip->base, BUFFSIZE); release_region(chip->io, DSP_NUMIO); free_irq(chip->irq, chip); diff --git a/sound/parisc/harmony.c b/sound/parisc/harmony.c index 29604a2..f2350c1 100644 --- a/sound/parisc/harmony.c +++ b/sound/parisc/harmony.c @@ -893,9 +893,7 @@ snd_harmony_free(struct snd_harmony *h) if (h->irq >= 0) free_irq(h->irq, h);
- if (h->iobase) - iounmap(h->iobase); - + iounmap(h->iobase); kfree(h); return 0; } diff --git a/sound/pci/ad1889.c b/sound/pci/ad1889.c index 547ee30..0de3129 100644 --- a/sound/pci/ad1889.c +++ b/sound/pci/ad1889.c @@ -853,12 +853,9 @@ snd_ad1889_free(struct snd_ad1889 *chip) free_irq(chip->irq, chip);
skip_hw: - if (chip->iobase) - iounmap(chip->iobase); - + iounmap(chip->iobase); pci_release_regions(chip->pci); pci_disable_device(chip->pci); - kfree(chip); return 0; } diff --git a/sound/pci/asihpi/hpioctl.c b/sound/pci/asihpi/hpioctl.c index 72af66b..67d1133 100644 --- a/sound/pci/asihpi/hpioctl.c +++ b/sound/pci/asihpi/hpioctl.c @@ -541,10 +541,8 @@ void asihpi_adapter_remove(struct pci_dev *pci_dev) hpi_send_recv_ex(&hm, &hr, HOWNER_KERNEL);
/* unmap PCI memory space, mapped during device init. */ - for (idx = 0; idx < HPI_MAX_ADAPTER_MEM_SPACES; idx++) { - if (pci.ap_mem_base[idx]) - iounmap(pci.ap_mem_base[idx]); - } + for (idx = 0; idx < HPI_MAX_ADAPTER_MEM_SPACES; ++idx) + iounmap(pci.ap_mem_base[idx]);
if (pa->irq) free_irq(pa->irq, pa); diff --git a/sound/pci/atiixp.c b/sound/pci/atiixp.c index 9c1c445..d24188f 100644 --- a/sound/pci/atiixp.c +++ b/sound/pci/atiixp.c @@ -1585,8 +1585,7 @@ static int snd_atiixp_free(struct atiixp *chip) __hw_end: if (chip->irq >= 0) free_irq(chip->irq, chip); - if (chip->remap_addr) - iounmap(chip->remap_addr); + iounmap(chip->remap_addr); pci_release_regions(chip->pci); pci_disable_device(chip->pci); kfree(chip); diff --git a/sound/pci/atiixp_modem.c b/sound/pci/atiixp_modem.c index b2f63e0..c321a97 100644 --- a/sound/pci/atiixp_modem.c +++ b/sound/pci/atiixp_modem.c @@ -1211,8 +1211,7 @@ static int snd_atiixp_free(struct atiixp_modem *chip) __hw_end: if (chip->irq >= 0) free_irq(chip->irq, chip); - if (chip->remap_addr) - iounmap(chip->remap_addr); + iounmap(chip->remap_addr); pci_release_regions(chip->pci); pci_disable_device(chip->pci); kfree(chip); diff --git a/sound/pci/aw2/aw2-alsa.c b/sound/pci/aw2/aw2-alsa.c index e1cf019..8d2fee7 100644 --- a/sound/pci/aw2/aw2-alsa.c +++ b/sound/pci/aw2/aw2-alsa.c @@ -229,9 +229,7 @@ static int snd_aw2_dev_free(struct snd_device *device) if (chip->irq >= 0) free_irq(chip->irq, (void *)chip); /* release the i/o ports & memory */ - if (chip->iobase_virt) - iounmap(chip->iobase_virt); - + iounmap(chip->iobase_virt); pci_release_regions(chip->pci); /* disable the PCI entry */ pci_disable_device(chip->pci); diff --git a/sound/pci/bt87x.c b/sound/pci/bt87x.c index 058b997..e82ceac 100644 --- a/sound/pci/bt87x.c +++ b/sound/pci/bt87x.c @@ -690,8 +690,7 @@ static int snd_bt87x_free(struct snd_bt87x *chip) snd_bt87x_stop(chip); if (chip->irq >= 0) free_irq(chip->irq, chip); - if (chip->mmio) - iounmap(chip->mmio); + iounmap(chip->mmio); pci_release_regions(chip->pci); pci_disable_device(chip->pci); kfree(chip); diff --git a/sound/pci/cs4281.c b/sound/pci/cs4281.c index 05a4337..ea33911 100644 --- a/sound/pci/cs4281.c +++ b/sound/pci/cs4281.c @@ -1316,10 +1316,8 @@ static int snd_cs4281_free(struct cs4281 *chip)
if (chip->irq >= 0) free_irq(chip->irq, chip); - if (chip->ba0) - iounmap(chip->ba0); - if (chip->ba1) - iounmap(chip->ba1); + iounmap(chip->ba0); + iounmap(chip->ba1); pci_release_regions(chip->pci); pci_disable_device(chip->pci);
diff --git a/sound/pci/cs46xx/cs46xx_lib.c b/sound/pci/cs46xx/cs46xx_lib.c index dfec84e..128bbfe 100644 --- a/sound/pci/cs46xx/cs46xx_lib.c +++ b/sound/pci/cs46xx/cs46xx_lib.c @@ -2949,8 +2949,8 @@ static int snd_cs46xx_free(struct snd_cs46xx *chip)
for (idx = 0; idx < 5; idx++) { struct snd_cs46xx_region *region = &chip->region.idx[idx]; - if (region->remap_addr) - iounmap(region->remap_addr); + + iounmap(region->remap_addr); release_and_free_resource(region->resource); }
diff --git a/sound/pci/ctxfi/cthw20k1.c b/sound/pci/ctxfi/cthw20k1.c index b425aa8..b8b0d8e 100644 --- a/sound/pci/ctxfi/cthw20k1.c +++ b/sound/pci/ctxfi/cthw20k1.c @@ -1985,10 +1985,7 @@ static int hw_card_shutdown(struct hw *hw) free_irq(hw->irq, hw);
hw->irq = -1; - - if (hw->mem_base) - iounmap(hw->mem_base); - + iounmap(hw->mem_base); hw->mem_base = NULL;
if (hw->io_base) diff --git a/sound/pci/ctxfi/cthw20k2.c b/sound/pci/ctxfi/cthw20k2.c index 253899d..4e16b4d 100644 --- a/sound/pci/ctxfi/cthw20k2.c +++ b/sound/pci/ctxfi/cthw20k2.c @@ -2110,10 +2110,7 @@ static int hw_card_shutdown(struct hw *hw) free_irq(hw->irq, hw);
hw->irq = -1; - - if (hw->mem_base) - iounmap(hw->mem_base); - + iounmap(hw->mem_base); hw->mem_base = NULL;
if (hw->io_base) diff --git a/sound/pci/echoaudio/echoaudio.c b/sound/pci/echoaudio/echoaudio.c index 21228ad..98d4f35 100644 --- a/sound/pci/echoaudio/echoaudio.c +++ b/sound/pci/echoaudio/echoaudio.c @@ -1872,12 +1872,8 @@ static int snd_echo_free(struct echoaudio *chip) if (chip->comm_page) snd_dma_free_pages(&chip->commpage_dma_buf);
- if (chip->dsp_registers) - iounmap(chip->dsp_registers); - + iounmap(chip->dsp_registers); release_and_free_resource(chip->iores); - - pci_disable_device(chip->pci);
/* release chip data */ diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c index d426a0b..a971425 100644 --- a/sound/pci/hda/hda_intel.c +++ b/sound/pci/hda/hda_intel.c @@ -1138,8 +1138,7 @@ static int azx_free(struct azx *chip) free_irq(chip->irq, (void*)chip); if (chip->msi) pci_disable_msi(chip->pci); - if (chip->remap_addr) - iounmap(chip->remap_addr); + iounmap(chip->remap_addr);
azx_free_stream_pages(chip); if (chip->region_requested) diff --git a/sound/pci/lola/lola.c b/sound/pci/lola/lola.c index 4cf4be5..9ff60008 100644 --- a/sound/pci/lola/lola.c +++ b/sound/pci/lola/lola.c @@ -551,10 +551,8 @@ static void lola_free(struct lola *chip) lola_free_mixer(chip); if (chip->irq >= 0) free_irq(chip->irq, (void *)chip); - if (chip->bar[0].remap_addr) - iounmap(chip->bar[0].remap_addr); - if (chip->bar[1].remap_addr) - iounmap(chip->bar[1].remap_addr); + iounmap(chip->bar[0].remap_addr); + iounmap(chip->bar[1].remap_addr); if (chip->rb.area) snd_dma_free_pages(&chip->rb); pci_release_regions(chip->pci); diff --git a/sound/pci/mixart/mixart.c b/sound/pci/mixart/mixart.c index 1faf47e..c3a9f39 100644 --- a/sound/pci/mixart/mixart.c +++ b/sound/pci/mixart/mixart.c @@ -1114,10 +1114,9 @@ static int snd_mixart_free(struct mixart_mgr *mgr) }
/* release the i/o ports */ - for (i = 0; i < 2; i++) { - if (mgr->mem[i].virt) - iounmap(mgr->mem[i].virt); - } + for (i = 0; i < 2; ++i) + iounmap(mgr->mem[i].virt); + pci_release_regions(mgr->pci);
/* free flowarray */ diff --git a/sound/pci/nm256/nm256.c b/sound/pci/nm256/nm256.c index 4e41a4e..3f52a44 100644 --- a/sound/pci/nm256/nm256.c +++ b/sound/pci/nm256/nm256.c @@ -1460,10 +1460,8 @@ static int snd_nm256_free(struct nm256 *chip) if (chip->irq >= 0) free_irq(chip->irq, chip);
- if (chip->cport) - iounmap(chip->cport); - if (chip->buffer) - iounmap(chip->buffer); + iounmap(chip->cport); + iounmap(chip->buffer); release_and_free_resource(chip->res_cport); release_and_free_resource(chip->res_buffer);
diff --git a/sound/pci/rme9652/hdsp.c b/sound/pci/rme9652/hdsp.c index cf5a6c8..fe66bcb 100644 --- a/sound/pci/rme9652/hdsp.c +++ b/sound/pci/rme9652/hdsp.c @@ -5309,9 +5309,7 @@ static int snd_hdsp_free(struct hdsp *hdsp)
release_firmware(hdsp->firmware); vfree(hdsp->fw_uploaded); - - if (hdsp->iobase) - iounmap(hdsp->iobase); + iounmap(hdsp->iobase);
if (hdsp->port) pci_release_regions(hdsp->pci); diff --git a/sound/pci/rme9652/hdspm.c b/sound/pci/rme9652/hdspm.c index 3342705..8109b8e 100644 --- a/sound/pci/rme9652/hdspm.c +++ b/sound/pci/rme9652/hdspm.c @@ -6965,9 +6965,7 @@ static int snd_hdspm_free(struct hdspm * hdspm) free_irq(hdspm->irq, (void *) hdspm);
kfree(hdspm->mixer); - - if (hdspm->iobase) - iounmap(hdspm->iobase); + iounmap(hdspm->iobase);
if (hdspm->port) pci_release_regions(hdspm->pci); diff --git a/sound/pci/rme9652/rme9652.c b/sound/pci/rme9652/rme9652.c index 6521521..648911c 100644 --- a/sound/pci/rme9652/rme9652.c +++ b/sound/pci/rme9652/rme9652.c @@ -1756,8 +1756,7 @@ static int snd_rme9652_free(struct snd_rme9652 *rme9652)
if (rme9652->irq >= 0) free_irq(rme9652->irq, (void *)rme9652); - if (rme9652->iobase) - iounmap(rme9652->iobase); + iounmap(rme9652->iobase); if (rme9652->port) pci_release_regions(rme9652->pci);
diff --git a/sound/pci/sis7019.c b/sound/pci/sis7019.c index 7f6a0a0..5e9437b 100644 --- a/sound/pci/sis7019.c +++ b/sound/pci/sis7019.c @@ -1064,12 +1064,9 @@ static int sis_chip_free(struct sis7019 *sis) if (sis->irq >= 0) free_irq(sis->irq, sis);
- if (sis->ioaddr) - iounmap(sis->ioaddr); - + iounmap(sis->ioaddr); pci_release_regions(sis->pci); pci_disable_device(sis->pci); - sis_free_suspend(sis); return 0; } diff --git a/sound/pci/ymfpci/ymfpci_main.c b/sound/pci/ymfpci/ymfpci_main.c index f5581a9..de7f06f 100644 --- a/sound/pci/ymfpci/ymfpci_main.c +++ b/sound/pci/ymfpci/ymfpci_main.c @@ -2246,8 +2246,7 @@ static int snd_ymfpci_free(struct snd_ymfpci *chip) release_and_free_resource(chip->mpu_res); release_and_free_resource(chip->fm_res); snd_ymfpci_free_gameport(chip); - if (chip->reg_area_virt) - iounmap(chip->reg_area_virt); + iounmap(chip->reg_area_virt); if (chip->work_ptr.area) snd_dma_free_pages(&chip->work_ptr); diff --git a/sound/ppc/pmac.c b/sound/ppc/pmac.c index 5a13b22..d399df4 100644 --- a/sound/ppc/pmac.c +++ b/sound/ppc/pmac.c @@ -867,16 +867,11 @@ static int snd_pmac_free(struct snd_pmac *chip) snd_pmac_dbdma_free(chip, &chip->capture.cmd); snd_pmac_dbdma_free(chip, &chip->extra_dma); snd_pmac_dbdma_free(chip, &emergency_dbdma); - if (chip->macio_base) - iounmap(chip->macio_base); - if (chip->latch_base) - iounmap(chip->latch_base); - if (chip->awacs) - iounmap(chip->awacs); - if (chip->playback.dma) - iounmap(chip->playback.dma); - if (chip->capture.dma) - iounmap(chip->capture.dma); + iounmap(chip->macio_base); + iounmap(chip->latch_base); + iounmap(chip->awacs); + iounmap(chip->playback.dma); + iounmap(chip->capture.dma);
if (chip->node) { int i;
On Sun, Jan 04, 2015 at 02:36:01PM +0100, SF Markus Elfring wrote:
/* unmap PCI memory space, mapped during device init. */
- for (idx = 0; idx < HPI_MAX_ADAPTER_MEM_SPACES; idx++) {
if (pci.ap_mem_base[idx])iounmap(pci.ap_mem_base[idx]);- }
- for (idx = 0; idx < HPI_MAX_ADAPTER_MEM_SPACES; ++idx)
iounmap(pci.ap_mem_base[idx]);
Don't do the gratuitous idx++ to ++idx changes. You do it a couple other places as well. It belongs in a separate patch if you really feel it is worth doing. (It is not a clean up and it is not worth doing).
regards, dan carpenter
Someone was just mentioning in another thread that removing the check from iounmap() is not portable to other arches and then I remembered that Markus removed a bunch of these.
We should consider reverting this, perhaps?
regards, dan carpenter
On Sun, Jan 04, 2015 at 02:36:01PM +0100, SF Markus Elfring wrote:
From: Markus Elfring elfring@users.sourceforge.net Date: Sat, 3 Jan 2015 22:55:54 +0100
The iounmap() function performs also input parameter validation. Thus the test around the call is not needed.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring elfring@users.sourceforge.net
sound/aoa/soundbus/i2sbus/core.c | 13 ++++++------- sound/arm/aaci.c | 4 ++-- sound/drivers/ml403-ac97cr.c | 3 +-- sound/isa/msnd/msnd_pinnacle.c | 3 +-- sound/parisc/harmony.c | 4 +--- sound/pci/ad1889.c | 5 +---- sound/pci/asihpi/hpioctl.c | 6 ++---- sound/pci/atiixp.c | 3 +-- sound/pci/atiixp_modem.c | 3 +-- sound/pci/aw2/aw2-alsa.c | 4 +--- sound/pci/bt87x.c | 3 +-- sound/pci/cs4281.c | 6 ++---- sound/pci/cs46xx/cs46xx_lib.c | 4 ++-- sound/pci/ctxfi/cthw20k1.c | 5 +---- sound/pci/ctxfi/cthw20k2.c | 5 +---- sound/pci/echoaudio/echoaudio.c | 6 +----- sound/pci/hda/hda_intel.c | 3 +-- sound/pci/lola/lola.c | 6 ++---- sound/pci/mixart/mixart.c | 7 +++---- sound/pci/nm256/nm256.c | 6 ++---- sound/pci/rme9652/hdsp.c | 4 +--- sound/pci/rme9652/hdspm.c | 4 +--- sound/pci/rme9652/rme9652.c | 3 +-- sound/pci/sis7019.c | 5 +---- sound/pci/ymfpci/ymfpci_main.c | 3 +-- sound/ppc/pmac.c | 15 +++++---------- 26 files changed, 43 insertions(+), 90 deletions(-)
diff --git a/sound/aoa/soundbus/i2sbus/core.c b/sound/aoa/soundbus/i2sbus/core.c index 4e2b4fb..7835045 100644 --- a/sound/aoa/soundbus/i2sbus/core.c +++ b/sound/aoa/soundbus/i2sbus/core.c @@ -74,10 +74,9 @@ static void i2sbus_release_dev(struct device *dev) int i;
i2sdev = container_of(dev, struct i2sbus_dev, sound.ofdev.dev);
- if (i2sdev->intfregs) iounmap(i2sdev->intfregs);
- if (i2sdev->out.dbdma) iounmap(i2sdev->out.dbdma);
- if (i2sdev->in.dbdma) iounmap(i2sdev->in.dbdma);
- iounmap(i2sdev->intfregs);
- iounmap(i2sdev->out.dbdma);
- iounmap(i2sdev->in.dbdma); for (i = aoa_resource_i2smmio; i <= aoa_resource_rxdbdma; i++) release_and_free_resource(i2sdev->allocated_resource[i]); free_dbdma_descriptor_ring(i2sdev, &i2sdev->out.dbdma_ring);
@@ -318,9 +317,9 @@ static int i2sbus_add_dev(struct macio_dev *macio, free_irq(dev->interrupts[i], dev); free_dbdma_descriptor_ring(dev, &dev->out.dbdma_ring); free_dbdma_descriptor_ring(dev, &dev->in.dbdma_ring);
- if (dev->intfregs) iounmap(dev->intfregs);
- if (dev->out.dbdma) iounmap(dev->out.dbdma);
- if (dev->in.dbdma) iounmap(dev->in.dbdma);
- iounmap(dev->intfregs);
- iounmap(dev->out.dbdma);
- iounmap(dev->in.dbdma); for (i=0;i<3;i++) release_and_free_resource(dev->allocated_resource[i]); mutex_destroy(&dev->lock);
diff --git a/sound/arm/aaci.c b/sound/arm/aaci.c index 0e83a73..4140b1b 100644 --- a/sound/arm/aaci.c +++ b/sound/arm/aaci.c @@ -889,8 +889,8 @@ static int aaci_probe_ac97(struct aaci *aaci) static void aaci_free_card(struct snd_card *card) { struct aaci *aaci = card->private_data;
- if (aaci->base)
iounmap(aaci->base);
- iounmap(aaci->base);
}
static struct aaci *aaci_init_card(struct amba_device *dev) diff --git a/sound/drivers/ml403-ac97cr.c b/sound/drivers/ml403-ac97cr.c index ec01de1..bdcb572 100644 --- a/sound/drivers/ml403-ac97cr.c +++ b/sound/drivers/ml403-ac97cr.c @@ -1094,8 +1094,7 @@ static int snd_ml403_ac97cr_free(struct snd_ml403_ac97cr *ml403_ac97cr) if (ml403_ac97cr->capture_irq >= 0) free_irq(ml403_ac97cr->capture_irq, ml403_ac97cr); /* give back "port" */
- if (ml403_ac97cr->port != NULL)
iounmap(ml403_ac97cr->port);
- iounmap(ml403_ac97cr->port); kfree(ml403_ac97cr); PDEBUG(INIT_INFO, "free(): (done)\n"); return 0;
diff --git a/sound/isa/msnd/msnd_pinnacle.c b/sound/isa/msnd/msnd_pinnacle.c index 65b3682..4c07266 100644 --- a/sound/isa/msnd/msnd_pinnacle.c +++ b/sound/isa/msnd/msnd_pinnacle.c @@ -627,8 +627,7 @@ static int snd_msnd_attach(struct snd_card *card) return 0;
err_release_region:
- if (chip->mappedbase)
iounmap(chip->mappedbase);
- iounmap(chip->mappedbase); release_mem_region(chip->base, BUFFSIZE); release_region(chip->io, DSP_NUMIO); free_irq(chip->irq, chip);
diff --git a/sound/parisc/harmony.c b/sound/parisc/harmony.c index 29604a2..f2350c1 100644 --- a/sound/parisc/harmony.c +++ b/sound/parisc/harmony.c @@ -893,9 +893,7 @@ snd_harmony_free(struct snd_harmony *h) if (h->irq >= 0) free_irq(h->irq, h);
- if (h->iobase)
iounmap(h->iobase);
- iounmap(h->iobase); kfree(h); return 0;
} diff --git a/sound/pci/ad1889.c b/sound/pci/ad1889.c index 547ee30..0de3129 100644 --- a/sound/pci/ad1889.c +++ b/sound/pci/ad1889.c @@ -853,12 +853,9 @@ snd_ad1889_free(struct snd_ad1889 *chip) free_irq(chip->irq, chip);
skip_hw:
- if (chip->iobase)
iounmap(chip->iobase);
- iounmap(chip->iobase); pci_release_regions(chip->pci); pci_disable_device(chip->pci);
- kfree(chip); return 0;
} diff --git a/sound/pci/asihpi/hpioctl.c b/sound/pci/asihpi/hpioctl.c index 72af66b..67d1133 100644 --- a/sound/pci/asihpi/hpioctl.c +++ b/sound/pci/asihpi/hpioctl.c @@ -541,10 +541,8 @@ void asihpi_adapter_remove(struct pci_dev *pci_dev) hpi_send_recv_ex(&hm, &hr, HOWNER_KERNEL);
/* unmap PCI memory space, mapped during device init. */
- for (idx = 0; idx < HPI_MAX_ADAPTER_MEM_SPACES; idx++) {
if (pci.ap_mem_base[idx])iounmap(pci.ap_mem_base[idx]);- }
for (idx = 0; idx < HPI_MAX_ADAPTER_MEM_SPACES; ++idx)
iounmap(pci.ap_mem_base[idx]);if (pa->irq) free_irq(pa->irq, pa);
diff --git a/sound/pci/atiixp.c b/sound/pci/atiixp.c index 9c1c445..d24188f 100644 --- a/sound/pci/atiixp.c +++ b/sound/pci/atiixp.c @@ -1585,8 +1585,7 @@ static int snd_atiixp_free(struct atiixp *chip) __hw_end: if (chip->irq >= 0) free_irq(chip->irq, chip);
- if (chip->remap_addr)
iounmap(chip->remap_addr);
- iounmap(chip->remap_addr); pci_release_regions(chip->pci); pci_disable_device(chip->pci); kfree(chip);
diff --git a/sound/pci/atiixp_modem.c b/sound/pci/atiixp_modem.c index b2f63e0..c321a97 100644 --- a/sound/pci/atiixp_modem.c +++ b/sound/pci/atiixp_modem.c @@ -1211,8 +1211,7 @@ static int snd_atiixp_free(struct atiixp_modem *chip) __hw_end: if (chip->irq >= 0) free_irq(chip->irq, chip);
- if (chip->remap_addr)
iounmap(chip->remap_addr);
- iounmap(chip->remap_addr); pci_release_regions(chip->pci); pci_disable_device(chip->pci); kfree(chip);
diff --git a/sound/pci/aw2/aw2-alsa.c b/sound/pci/aw2/aw2-alsa.c index e1cf019..8d2fee7 100644 --- a/sound/pci/aw2/aw2-alsa.c +++ b/sound/pci/aw2/aw2-alsa.c @@ -229,9 +229,7 @@ static int snd_aw2_dev_free(struct snd_device *device) if (chip->irq >= 0) free_irq(chip->irq, (void *)chip); /* release the i/o ports & memory */
- if (chip->iobase_virt)
iounmap(chip->iobase_virt);
- iounmap(chip->iobase_virt); pci_release_regions(chip->pci); /* disable the PCI entry */ pci_disable_device(chip->pci);
diff --git a/sound/pci/bt87x.c b/sound/pci/bt87x.c index 058b997..e82ceac 100644 --- a/sound/pci/bt87x.c +++ b/sound/pci/bt87x.c @@ -690,8 +690,7 @@ static int snd_bt87x_free(struct snd_bt87x *chip) snd_bt87x_stop(chip); if (chip->irq >= 0) free_irq(chip->irq, chip);
- if (chip->mmio)
iounmap(chip->mmio);
- iounmap(chip->mmio); pci_release_regions(chip->pci); pci_disable_device(chip->pci); kfree(chip);
diff --git a/sound/pci/cs4281.c b/sound/pci/cs4281.c index 05a4337..ea33911 100644 --- a/sound/pci/cs4281.c +++ b/sound/pci/cs4281.c @@ -1316,10 +1316,8 @@ static int snd_cs4281_free(struct cs4281 *chip)
if (chip->irq >= 0) free_irq(chip->irq, chip);
- if (chip->ba0)
iounmap(chip->ba0);- if (chip->ba1)
iounmap(chip->ba1);
- iounmap(chip->ba0);
- iounmap(chip->ba1); pci_release_regions(chip->pci); pci_disable_device(chip->pci);
diff --git a/sound/pci/cs46xx/cs46xx_lib.c b/sound/pci/cs46xx/cs46xx_lib.c index dfec84e..128bbfe 100644 --- a/sound/pci/cs46xx/cs46xx_lib.c +++ b/sound/pci/cs46xx/cs46xx_lib.c @@ -2949,8 +2949,8 @@ static int snd_cs46xx_free(struct snd_cs46xx *chip)
for (idx = 0; idx < 5; idx++) { struct snd_cs46xx_region *region = &chip->region.idx[idx];
if (region->remap_addr)iounmap(region->remap_addr);
iounmap(region->remap_addr);diff --git a/sound/pci/ctxfi/cthw20k1.c b/sound/pci/ctxfi/cthw20k1.c index b425aa8..b8b0d8e 100644 --- a/sound/pci/ctxfi/cthw20k1.c +++ b/sound/pci/ctxfi/cthw20k1.c @@ -1985,10 +1985,7 @@ static int hw_card_shutdown(struct hw *hw) free_irq(hw->irq, hw);
hw->irq = -1;
- if (hw->mem_base)
iounmap(hw->mem_base);
iounmap(hw->mem_base); hw->mem_base = NULL;
if (hw->io_base)
diff --git a/sound/pci/ctxfi/cthw20k2.c b/sound/pci/ctxfi/cthw20k2.c index 253899d..4e16b4d 100644 --- a/sound/pci/ctxfi/cthw20k2.c +++ b/sound/pci/ctxfi/cthw20k2.c @@ -2110,10 +2110,7 @@ static int hw_card_shutdown(struct hw *hw) free_irq(hw->irq, hw);
hw->irq = -1;
- if (hw->mem_base)
iounmap(hw->mem_base);
iounmap(hw->mem_base); hw->mem_base = NULL;
if (hw->io_base)
diff --git a/sound/pci/echoaudio/echoaudio.c b/sound/pci/echoaudio/echoaudio.c index 21228ad..98d4f35 100644 --- a/sound/pci/echoaudio/echoaudio.c +++ b/sound/pci/echoaudio/echoaudio.c @@ -1872,12 +1872,8 @@ static int snd_echo_free(struct echoaudio *chip) if (chip->comm_page) snd_dma_free_pages(&chip->commpage_dma_buf);
- if (chip->dsp_registers)
iounmap(chip->dsp_registers);
- iounmap(chip->dsp_registers); release_and_free_resource(chip->iores);
pci_disable_device(chip->pci);
/* release chip data */
diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c index d426a0b..a971425 100644 --- a/sound/pci/hda/hda_intel.c +++ b/sound/pci/hda/hda_intel.c @@ -1138,8 +1138,7 @@ static int azx_free(struct azx *chip) free_irq(chip->irq, (void*)chip); if (chip->msi) pci_disable_msi(chip->pci);
- if (chip->remap_addr)
iounmap(chip->remap_addr);
iounmap(chip->remap_addr);
azx_free_stream_pages(chip); if (chip->region_requested)
diff --git a/sound/pci/lola/lola.c b/sound/pci/lola/lola.c index 4cf4be5..9ff60008 100644 --- a/sound/pci/lola/lola.c +++ b/sound/pci/lola/lola.c @@ -551,10 +551,8 @@ static void lola_free(struct lola *chip) lola_free_mixer(chip); if (chip->irq >= 0) free_irq(chip->irq, (void *)chip);
- if (chip->bar[0].remap_addr)
iounmap(chip->bar[0].remap_addr);- if (chip->bar[1].remap_addr)
iounmap(chip->bar[1].remap_addr);
- iounmap(chip->bar[0].remap_addr);
- iounmap(chip->bar[1].remap_addr); if (chip->rb.area) snd_dma_free_pages(&chip->rb); pci_release_regions(chip->pci);
diff --git a/sound/pci/mixart/mixart.c b/sound/pci/mixart/mixart.c index 1faf47e..c3a9f39 100644 --- a/sound/pci/mixart/mixart.c +++ b/sound/pci/mixart/mixart.c @@ -1114,10 +1114,9 @@ static int snd_mixart_free(struct mixart_mgr *mgr) }
/* release the i/o ports */
- for (i = 0; i < 2; i++) {
if (mgr->mem[i].virt)iounmap(mgr->mem[i].virt);- }
for (i = 0; i < 2; ++i)
iounmap(mgr->mem[i].virt);pci_release_regions(mgr->pci);
/* free flowarray */
diff --git a/sound/pci/nm256/nm256.c b/sound/pci/nm256/nm256.c index 4e41a4e..3f52a44 100644 --- a/sound/pci/nm256/nm256.c +++ b/sound/pci/nm256/nm256.c @@ -1460,10 +1460,8 @@ static int snd_nm256_free(struct nm256 *chip) if (chip->irq >= 0) free_irq(chip->irq, chip);
- if (chip->cport)
iounmap(chip->cport);- if (chip->buffer)
iounmap(chip->buffer);
- iounmap(chip->cport);
- iounmap(chip->buffer); release_and_free_resource(chip->res_cport); release_and_free_resource(chip->res_buffer);
diff --git a/sound/pci/rme9652/hdsp.c b/sound/pci/rme9652/hdsp.c index cf5a6c8..fe66bcb 100644 --- a/sound/pci/rme9652/hdsp.c +++ b/sound/pci/rme9652/hdsp.c @@ -5309,9 +5309,7 @@ static int snd_hdsp_free(struct hdsp *hdsp)
release_firmware(hdsp->firmware); vfree(hdsp->fw_uploaded);
- if (hdsp->iobase)
iounmap(hdsp->iobase);
iounmap(hdsp->iobase);
if (hdsp->port) pci_release_regions(hdsp->pci);
diff --git a/sound/pci/rme9652/hdspm.c b/sound/pci/rme9652/hdspm.c index 3342705..8109b8e 100644 --- a/sound/pci/rme9652/hdspm.c +++ b/sound/pci/rme9652/hdspm.c @@ -6965,9 +6965,7 @@ static int snd_hdspm_free(struct hdspm * hdspm) free_irq(hdspm->irq, (void *) hdspm);
kfree(hdspm->mixer);
- if (hdspm->iobase)
iounmap(hdspm->iobase);
iounmap(hdspm->iobase);
if (hdspm->port) pci_release_regions(hdspm->pci);
diff --git a/sound/pci/rme9652/rme9652.c b/sound/pci/rme9652/rme9652.c index 6521521..648911c 100644 --- a/sound/pci/rme9652/rme9652.c +++ b/sound/pci/rme9652/rme9652.c @@ -1756,8 +1756,7 @@ static int snd_rme9652_free(struct snd_rme9652 *rme9652)
if (rme9652->irq >= 0) free_irq(rme9652->irq, (void *)rme9652);
- if (rme9652->iobase)
iounmap(rme9652->iobase);
- iounmap(rme9652->iobase); if (rme9652->port) pci_release_regions(rme9652->pci);
diff --git a/sound/pci/sis7019.c b/sound/pci/sis7019.c index 7f6a0a0..5e9437b 100644 --- a/sound/pci/sis7019.c +++ b/sound/pci/sis7019.c @@ -1064,12 +1064,9 @@ static int sis_chip_free(struct sis7019 *sis) if (sis->irq >= 0) free_irq(sis->irq, sis);
- if (sis->ioaddr)
iounmap(sis->ioaddr);
- iounmap(sis->ioaddr); pci_release_regions(sis->pci); pci_disable_device(sis->pci);
- sis_free_suspend(sis); return 0;
} diff --git a/sound/pci/ymfpci/ymfpci_main.c b/sound/pci/ymfpci/ymfpci_main.c index f5581a9..de7f06f 100644 --- a/sound/pci/ymfpci/ymfpci_main.c +++ b/sound/pci/ymfpci/ymfpci_main.c @@ -2246,8 +2246,7 @@ static int snd_ymfpci_free(struct snd_ymfpci *chip) release_and_free_resource(chip->mpu_res); release_and_free_resource(chip->fm_res); snd_ymfpci_free_gameport(chip);
- if (chip->reg_area_virt)
iounmap(chip->reg_area_virt);
- iounmap(chip->reg_area_virt); if (chip->work_ptr.area) snd_dma_free_pages(&chip->work_ptr);
diff --git a/sound/ppc/pmac.c b/sound/ppc/pmac.c index 5a13b22..d399df4 100644 --- a/sound/ppc/pmac.c +++ b/sound/ppc/pmac.c @@ -867,16 +867,11 @@ static int snd_pmac_free(struct snd_pmac *chip) snd_pmac_dbdma_free(chip, &chip->capture.cmd); snd_pmac_dbdma_free(chip, &chip->extra_dma); snd_pmac_dbdma_free(chip, &emergency_dbdma);
- if (chip->macio_base)
iounmap(chip->macio_base);- if (chip->latch_base)
iounmap(chip->latch_base);- if (chip->awacs)
iounmap(chip->awacs);- if (chip->playback.dma)
iounmap(chip->playback.dma);- if (chip->capture.dma)
iounmap(chip->capture.dma);
iounmap(chip->macio_base);
iounmap(chip->latch_base);
iounmap(chip->awacs);
iounmap(chip->playback.dma);
iounmap(chip->capture.dma);
if (chip->node) { int i;
-- 2.2.1
-- To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, 2016-10-26 at 15:26 +0300, Dan Carpenter wrote:
Someone was just mentioning in another thread that removing the check from iounmap() is not portable to other arches and then I remembered that Markus removed a bunch of these.
We should consider reverting this, perhaps?
Can't we teach all architectures? Not that reverting it would be a problem.
johannes
On Wed, Oct 26, 2016 at 02:28:59PM +0200, Johannes Berg wrote:
On Wed, 2016-10-26 at 15:26 +0300, Dan Carpenter wrote:
Someone was just mentioning in another thread that removing the check from iounmap() is not portable to other arches and then I remembered that Markus removed a bunch of these.
We should consider reverting this, perhaps?
Can't we teach all architectures? Not that reverting it would be a problem.
We probably should. I just didn't want to suggest it, in case it sounds like volunteering... I'm not going to be able to do it because I'm on the road for a bit.
regards, dan carpenter
From: Markus Elfring elfring@users.sourceforge.net Date: Sun, 4 Jan 2015 11:00:11 +0100
The iounmap() function was called in one case by the snd_msnd_attach() function even if a previous call of the ioremap_nocache() function failed.
This implementation detail could be improved by the introduction of another jump label.
Signed-off-by: Markus Elfring elfring@users.sourceforge.net --- sound/isa/msnd/msnd_pinnacle.c | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-)
diff --git a/sound/isa/msnd/msnd_pinnacle.c b/sound/isa/msnd/msnd_pinnacle.c index 4c07266..e2e940d 100644 --- a/sound/isa/msnd/msnd_pinnacle.c +++ b/sound/isa/msnd/msnd_pinnacle.c @@ -575,23 +575,23 @@ static int snd_msnd_attach(struct snd_card *card)
err = snd_msnd_dsp_full_reset(card); if (err < 0) - goto err_release_region; + goto io_unmap;
/* Register device */ err = snd_device_new(card, SNDRV_DEV_LOWLEVEL, chip, &ops); if (err < 0) - goto err_release_region; + goto io_unmap;
err = snd_msnd_pcm(card, 0); if (err < 0) { printk(KERN_ERR LOGNAME ": error creating new PCM device\n"); - goto err_release_region; + goto io_unmap; }
err = snd_msndmix_new(card); if (err < 0) { printk(KERN_ERR LOGNAME ": error creating new Mixer device\n"); - goto err_release_region; + goto io_unmap; }
@@ -607,7 +607,7 @@ static int snd_msnd_attach(struct snd_card *card) if (err < 0) { printk(KERN_ERR LOGNAME ": error creating new Midi device\n"); - goto err_release_region; + goto io_unmap; } mpu = chip->rmidi->private_data;
@@ -622,12 +622,13 @@ static int snd_msnd_attach(struct snd_card *card)
err = snd_card_register(card); if (err < 0) - goto err_release_region; + goto io_unmap;
return 0;
-err_release_region: +io_unmap: iounmap(chip->mappedbase); +err_release_region: release_mem_region(chip->base, BUFFSIZE); release_region(chip->io, DSP_NUMIO); free_irq(chip->irq, chip);
At Sun, 04 Jan 2015 14:38:38 +0100, SF Markus Elfring wrote:
From: Markus Elfring elfring@users.sourceforge.net Date: Sun, 4 Jan 2015 11:00:11 +0100
The iounmap() function was called in one case by the snd_msnd_attach() function even if a previous call of the ioremap_nocache() function failed.
This implementation detail could be improved by the introduction of another jump label.
Signed-off-by: Markus Elfring elfring@users.sourceforge.net
I didn't apply this and the next one. Using goto label isn't bad for the error paths in general, but using too much nested labels worsens the readability significantly. (Though, it's almost a matter of taste, so I don't want to spend my time for discussing this.)
thanks,
Takashi
sound/isa/msnd/msnd_pinnacle.c | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-)
diff --git a/sound/isa/msnd/msnd_pinnacle.c b/sound/isa/msnd/msnd_pinnacle.c index 4c07266..e2e940d 100644 --- a/sound/isa/msnd/msnd_pinnacle.c +++ b/sound/isa/msnd/msnd_pinnacle.c @@ -575,23 +575,23 @@ static int snd_msnd_attach(struct snd_card *card)
err = snd_msnd_dsp_full_reset(card); if (err < 0)
goto err_release_region;
goto io_unmap;/* Register device */ err = snd_device_new(card, SNDRV_DEV_LOWLEVEL, chip, &ops); if (err < 0)
goto err_release_region;
goto io_unmap;err = snd_msnd_pcm(card, 0); if (err < 0) { printk(KERN_ERR LOGNAME ": error creating new PCM device\n");
goto err_release_region;
goto io_unmap;}
err = snd_msndmix_new(card); if (err < 0) { printk(KERN_ERR LOGNAME ": error creating new Mixer device\n");
goto err_release_region;
goto io_unmap;@@ -607,7 +607,7 @@ static int snd_msnd_attach(struct snd_card *card) if (err < 0) { printk(KERN_ERR LOGNAME ": error creating new Midi device\n");
goto err_release_region;
goto io_unmap;@@ -622,12 +622,13 @@ static int snd_msnd_attach(struct snd_card *card)
err = snd_card_register(card); if (err < 0)
goto err_release_region;
goto io_unmap;return 0;
-err_release_region: +io_unmap: iounmap(chip->mappedbase); +err_release_region: release_mem_region(chip->base, BUFFSIZE); release_region(chip->io, DSP_NUMIO); free_irq(chip->irq, chip); -- 2.2.1
From: Markus Elfring elfring@users.sourceforge.net Date: Sun, 4 Jan 2015 11:47:17 +0100
Two return statements were used by the snd_msnd_attach() function at source code places where the Linux coding style recommends an alternative approach.
Let us improve the affected implementation details with adjustments for corresponding jump targets.
Signed-off-by: Markus Elfring elfring@users.sourceforge.net --- sound/isa/msnd/msnd_pinnacle.c | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-)
diff --git a/sound/isa/msnd/msnd_pinnacle.c b/sound/isa/msnd/msnd_pinnacle.c index e2e940d..0e66e5e 100644 --- a/sound/isa/msnd/msnd_pinnacle.c +++ b/sound/isa/msnd/msnd_pinnacle.c @@ -552,17 +552,16 @@ static int snd_msnd_attach(struct snd_card *card) return err; } if (request_region(chip->io, DSP_NUMIO, card->shortname) == NULL) { - free_irq(chip->irq, chip); - return -EBUSY; + err = -EBUSY; + goto free_an_irq; }
if (!request_mem_region(chip->base, BUFFSIZE, card->shortname)) { printk(KERN_ERR LOGNAME ": unable to grab memory region 0x%lx-0x%lx\n", chip->base, chip->base + BUFFSIZE - 1); - release_region(chip->io, DSP_NUMIO); - free_irq(chip->irq, chip); - return -EBUSY; + err = -EBUSY; + goto release_resource_region; } chip->mappedbase = ioremap_nocache(chip->base, 0x8000); if (!chip->mappedbase) { @@ -570,7 +569,7 @@ static int snd_msnd_attach(struct snd_card *card) ": unable to map memory region 0x%lx-0x%lx\n", chip->base, chip->base + BUFFSIZE - 1); err = -EIO; - goto err_release_region; + goto release_memory_region; }
err = snd_msnd_dsp_full_reset(card); @@ -628,9 +627,11 @@ static int snd_msnd_attach(struct snd_card *card)
io_unmap: iounmap(chip->mappedbase); -err_release_region: +release_memory_region: release_mem_region(chip->base, BUFFSIZE); +release_resource_region: release_region(chip->io, DSP_NUMIO); +free_an_irq: free_irq(chip->irq, chip); return err; }
At Sun, 04 Jan 2015 13:43:11 +0100, SF Markus Elfring wrote:
From: Markus Elfring elfring@users.sourceforge.net Date: Sun, 4 Jan 2015 11:50:12 +0100
Further update suggestions were taken into account after several patches were applied from static source code analysis.
Markus Elfring (3): seq: Deletion of unnecessary checks before the function call "snd_midi_event_free" oss: Deletion of unnecessary checks before the function call "vfree" emu10k1: Delete an unnecessary check before the function call "kfree" oxygen: Delete an unnecessary check before the function call "snd_pcm_suspend" emux: Delete an unnecessary check before the function call "snd_sf_free" ASoC: Intel: Delete an unnecessary check before the function call "sst_dma_free" ASoC: fsi: Deletion of unnecessary checks before the function call "clk_enable" ASoC: Intel: Delete an unnecessary check before the function call "release_firmware" i2sbus: Delete an unnecessary check before the function call "snd_pcm_suspend_all" sb: Delete an unnecessary check before the function call "snd_emux_free" Deletion of checks before the function call "iounmap" msnd: One function call less in snd_msnd_attach() after error detection msnd: Fix centralized exiting from snd_msnd_attach()
I applied most of them now but leave patches 6-8 left to Mark applying through his tree. As posted in another mail patches 12 and 13 are dropped.
BTW, when you sending a new patch series, you should drop the old references tags.
thanks,
Takashi
sound/aoa/soundbus/i2sbus/core.c | 13 ++++++------- sound/arm/aaci.c | 4 ++-- sound/drivers/ml403-ac97cr.c | 3 +-- sound/isa/msnd/msnd_pinnacle.c | 31 ++++++++++++++++--------------- sound/parisc/harmony.c | 4 +--- sound/pci/ad1889.c | 5 +---- sound/pci/asihpi/hpioctl.c | 6 ++---- sound/pci/atiixp.c | 3 +-- sound/pci/atiixp_modem.c | 3 +-- sound/pci/aw2/aw2-alsa.c | 4 +--- sound/pci/bt87x.c | 3 +-- sound/pci/cs4281.c | 6 ++---- sound/pci/cs46xx/cs46xx_lib.c | 4 ++-- sound/pci/ctxfi/cthw20k1.c | 5 +---- sound/pci/ctxfi/cthw20k2.c | 5 +---- sound/pci/echoaudio/echoaudio.c | 6 +----- sound/pci/hda/hda_intel.c | 3 +-- sound/pci/lola/lola.c | 6 ++---- sound/pci/mixart/mixart.c | 7 +++---- sound/pci/nm256/nm256.c | 6 ++---- sound/pci/rme9652/hdsp.c | 4 +--- sound/pci/rme9652/hdspm.c | 4 +--- sound/pci/rme9652/rme9652.c | 3 +-- sound/pci/sis7019.c | 5 +---- sound/pci/ymfpci/ymfpci_main.c | 3 +-- sound/ppc/pmac.c | 15 +++++---------- 26 files changed, 58 insertions(+), 103 deletions(-)
-- 2.2.1
participants (5)
-
Dan Carpenter -
Johannes Berg -
Mark Brown -
SF Markus Elfring -
Takashi Iwai