Re: [PATCH 11/13] ALSA: Deletion of checks before the function call "iounmap"

[Dan Carpenter]

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

Re: [PATCH 11/13] ALSA: Deletion of checks before the function call "iounmap"

[Johannes Berg]

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

Re: [PATCH 11/13] ALSA: Deletion of checks before the function call "iounmap"

[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 
> 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 
> ---
>  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, >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, >out.dbdma_ring);
>   free_dbdma_descriptor_ring(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(>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);
> 

Re: [PATCH 11/13] ALSA: Deletion of checks before the function call iounmap

[Dan Carpenter]

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

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH 11/13] ALSA: Deletion of checks before the function call iounmap

[SF Markus Elfring]

From: Markus Elfring elfr...@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 elfr...@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;i3;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