[PATCH 1/1] tty-hvsi_lib: Deletion of an unnecessary check before the function call "tty_kref_put"
From: Markus Elfring Date: Fri, 21 Nov 2014 12:40:32 +0100 The tty_kref_put() 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 --- drivers/tty/hvc/hvsi_lib.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/tty/hvc/hvsi_lib.c b/drivers/tty/hvc/hvsi_lib.c index 7ae6c29..a270f04 100644 --- a/drivers/tty/hvc/hvsi_lib.c +++ b/drivers/tty/hvc/hvsi_lib.c @@ -405,8 +405,7 @@ void hvsilib_close(struct hvsi_priv *pv, struct hvc_struct *hp) hvsi_send_close(pv); } - if (pv->tty) - tty_kref_put(pv->tty); + tty_kref_put(pv->tty); pv->tty = NULL; } -- 2.1.3 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 1/1] PowerPC-83xx: Deletion of an unnecessary check before the function call "of_node_put"
From: Markus Elfring Date: Sat, 22 Nov 2014 16:18:20 +0100 The of_node_put() 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 --- arch/powerpc/platforms/83xx/usb.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/arch/powerpc/platforms/83xx/usb.c b/arch/powerpc/platforms/83xx/usb.c index 1ad748b..5c31d82 100644 --- a/arch/powerpc/platforms/83xx/usb.c +++ b/arch/powerpc/platforms/83xx/usb.c @@ -162,8 +162,7 @@ int mpc831x_usb_cfg(void) iounmap(immap); - if (immr_node) - of_node_put(immr_node); + of_node_put(immr_node); /* Map USB SOC space */ ret = of_address_to_resource(np, 0, &res); -- 2.1.3 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH] ALSA: i2sbus: Deletion of unnecessary checks before the function call "release_and_free_resource"
From: Markus Elfring Date: Tue, 2 Dec 2014 22:50:24 +0100 The release_and_free_resource() 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 --- 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 a80d5ea..4e2b4fb 100644 --- a/sound/aoa/soundbus/i2sbus/core.c +++ b/sound/aoa/soundbus/i2sbus/core.c @@ -79,8 +79,7 @@ static void i2sbus_release_dev(struct device *dev) if (i2sdev->out.dbdma) iounmap(i2sdev->out.dbdma); if (i2sdev->in.dbdma) iounmap(i2sdev->in.dbdma); for (i = aoa_resource_i2smmio; i <= aoa_resource_rxdbdma; i++) - if (i2sdev->allocated_resource[i]) - release_and_free_resource(i2sdev->allocated_resource[i]); + release_and_free_resource(i2sdev->allocated_resource[i]); free_dbdma_descriptor_ring(i2sdev, &i2sdev->out.dbdma_ring); free_dbdma_descriptor_ring(i2sdev, &i2sdev->in.dbdma_ring); for (i = aoa_resource_i2smmio; i <= aoa_resource_rxdbdma; i++) @@ -323,8 +322,7 @@ static int i2sbus_add_dev(struct macio_dev *macio, if (dev->out.dbdma) iounmap(dev->out.dbdma); if (dev->in.dbdma) iounmap(dev->in.dbdma); for (i=0;i<3;i++) - if (dev->allocated_resource[i]) - release_and_free_resource(dev->allocated_resource[i]); + release_and_free_resource(dev->allocated_resource[i]); mutex_destroy(&dev->lock); kfree(dev); return 0; -- 2.1.3 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 9/13] ALSA: i2sbus: Delete an unnecessary check before the function call "snd_pcm_suspend_all"
From: Markus Elfring
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
---
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) {
--
2.2.1
___
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"
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, &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/soun
Re: [PATCH] macintosh: Delete an unnecessary check before the function call "of_node_put"
Am 04.02.2015 um 21:36 schrieb SF Markus Elfring: > From: Markus Elfring > Date: Wed, 4 Feb 2015 21:32:27 +0100 > > The of_node_put() 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 > --- > drivers/macintosh/smu.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/drivers/macintosh/smu.c b/drivers/macintosh/smu.c > index 10ae69b..d531f80 100644 > --- a/drivers/macintosh/smu.c > +++ b/drivers/macintosh/smu.c > @@ -557,8 +557,7 @@ int __init smu_init (void) > return 0; > > fail_msg_node: > - if (smu->msg_node) > - of_node_put(smu->msg_node); > + of_node_put(smu->msg_node); > fail_db_node: > of_node_put(smu->db_node); > fail_bootmem: > Would you like to integrate this update suggestion into another source code repository? Regards, Markus ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH] PowerPC-PCI: Delete unnecessary checks before the function call "kfree"
From: Markus Elfring
Date: Tue, 3 Feb 2015 13:55:53 +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
---
arch/powerpc/platforms/cell/celleb_pci.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/platforms/cell/celleb_pci.c
b/arch/powerpc/platforms/cell/celleb_pci.c
index 3ce70de..9b11b5d 100644
--- a/arch/powerpc/platforms/cell/celleb_pci.c
+++ b/arch/powerpc/platforms/cell/celleb_pci.c
@@ -393,11 +393,10 @@ static int __init celleb_setup_fake_pci_device(struct
device_node *node,
error:
if (mem_init_done) {
- if (config && *config)
+ if (config)
kfree(*config);
- if (res && *res)
+ if (res)
kfree(*res);
-
} else {
if (config && *config) {
size = 256;
--
2.2.2
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH] PowerPC-rheap: Delete an unnecessary check before the function call "kfree"
From: Markus Elfring
Date: Tue, 3 Feb 2015 14:34:10 +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
---
arch/powerpc/lib/rheap.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/lib/rheap.c b/arch/powerpc/lib/rheap.c
index a1060a8..69abf84 100644
--- a/arch/powerpc/lib/rheap.c
+++ b/arch/powerpc/lib/rheap.c
@@ -284,7 +284,7 @@ EXPORT_SYMBOL_GPL(rh_create);
*/
void rh_destroy(rh_info_t * info)
{
- if ((info->flags & RHIF_STATIC_BLOCK) == 0 && info->block != NULL)
+ if ((info->flags & RHIF_STATIC_BLOCK) == 0)
kfree(info->block);
if ((info->flags & RHIF_STATIC_INFO) == 0)
--
2.2.2
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH] macintosh: Delete an unnecessary check before the function call "of_node_put"
From: Markus Elfring Date: Wed, 4 Feb 2015 21:32:27 +0100 The of_node_put() 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 --- drivers/macintosh/smu.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/macintosh/smu.c b/drivers/macintosh/smu.c index 10ae69b..d531f80 100644 --- a/drivers/macintosh/smu.c +++ b/drivers/macintosh/smu.c @@ -557,8 +557,7 @@ int __init smu_init (void) return 0; fail_msg_node: - if (smu->msg_node) - of_node_put(smu->msg_node); + of_node_put(smu->msg_node); fail_db_node: of_node_put(smu->db_node); fail_bootmem: -- 2.2.2 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH] cxl: Delete an unnecessary check before the function call "kfree"
From: Markus Elfring Date: Fri, 6 Nov 2015 11:00:23 +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 --- drivers/misc/cxl/context.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/misc/cxl/context.c b/drivers/misc/cxl/context.c index 2faa127..52e39b6 100644 --- a/drivers/misc/cxl/context.c +++ b/drivers/misc/cxl/context.c @@ -275,9 +275,7 @@ static void reclaim_ctx(struct rcu_head *rcu) if (ctx->kernelapi) kfree(ctx->mapping); - if (ctx->irq_bitmap) - kfree(ctx->irq_bitmap); - + kfree(ctx->irq_bitmap); kfree(ctx); } -- 2.6.2 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[POWERPC] bootwrapper: One check less in fsl_get_immr() after error detection
From: Markus Elfring
Date: Mon, 14 Dec 2015 23:01:32 +0100
A status check was performed by the fsl_get_immr() function even if it
was known already that a system setting did not fit to the expectations.
This implementation detail could be improved by an adjustment for
a jump label according to the Linux coding style convention.
Signed-off-by: Markus Elfring
---
arch/powerpc/boot/fsl-soc.c | 14 +++---
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/arch/powerpc/boot/fsl-soc.c b/arch/powerpc/boot/fsl-soc.c
index b835ed6..ff1dae3 100644
--- a/arch/powerpc/boot/fsl-soc.c
+++ b/arch/powerpc/boot/fsl-soc.c
@@ -34,24 +34,24 @@ u32 *fsl_get_immr(void)
naddr = 2;
if (naddr != 1 && naddr != 2)
- goto err;
+ goto report_failure;
size = getprop(soc, "ranges", prop_buf, MAX_PROP_LEN);
if (size < 12)
- goto err;
+ goto report_failure;
if (prop_buf[0] != 0)
- goto err;
+ goto report_failure;
if (naddr == 2 && prop_buf[1] != 0)
- goto err;
+ goto report_failure;
if (!dt_xlate_addr(soc, prop_buf + naddr, 8, &ret))
ret = 0;
}
-err:
- if (!ret)
+ if (!ret) {
+report_failure:
printf("fsl_get_immr: Failed to find immr base\r\n");
-
+ }
return (u32 *)ret;
}
--
2.6.3
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH RFC 3/5] coccinelle: add xxxsetbitsXX converting spatch
How do you think about to add any more meta-data for this semantic patch script? * SPDX identifier * Copyright information * Confidence level https://bottest.wiki.kernel.org/coccicheck > +virtual context Further variables will be needed if you would like to use corresponding operation modes (besides “patch”). > +expression reg; > +expression set; > +expression clear; I propose once more to avoid the repetition of (unnecessary) SmPL code. This part could be written like the following instead. +expression clear, set, reg; If you would increase the usage of SmPL disjunctions, the specification of duplicate SmPL code could be reduced considerably. * Would you like to merge SmPL rules based on the distinction between the data types “u32” and “u64”? * Did you identify any optional code in this transformation approach? > +@@ > +expression base; > +expression offset; > +expression value; > +@@ > + > +- mtu3_setbits(base, offset, value); > ++ setbits32(base + offset, value); > + > +@@ > +expression base; > +expression offset; > +expression mask; > +@@ > + > +- mtu3_clrbits(base, offset, mask); > ++ clrbits32(base + offset, mask); Another update suggestion: +@replacement@ +expression base, offset; +@@ +( +-mtu3_clrbits ++clrbits32 +| +-mtu3_setbits ++setbits32 +)(base +- , ++ + + offset, ...); Would you like to try further software fine-tuning out? Regards, Markus
Re: [PATCH v2 1/2] treewide: remove unused address argument from pte_alloc functions
> The changes were obtained by applying the following Coccinelle script. A bit of clarification happened for its implementation details. https://systeme.lip6.fr/pipermail/cocci/2018-October/005374.html I have taken also another look at the following SmPL code. > identifier fn =~ > "^(__pte_alloc|pte_alloc_one|pte_alloc|__pte_alloc_kernel|pte_alloc_one_kernel)$"; I suggest to adjust the regular expression for this constraint and in subsequent SmPL rules. "^(?:pte_alloc(?:_one(?:_kernel)?)?|__pte_alloc(?:_kernel)?)$"; > ( > - T3 fn(T1 E1, T2 E2); > + T3 fn(T1 E1); > | > - T3 fn(T1 E1, T2 E2, T4 E4); > + T3 fn(T1 E1, T2 E2); > ) I propose to take an other SmPL disjunction into account here. T3 fn(T1 E1, ( - T2 E2 | T2 E2, - T4 E4 ) ); > ( > - #define fn(a, b, c)@p e > + #define fn(a, b) e > | > - #define fn(a, b)@p e > + #define fn(a) e > ) How do you think about to omit the metavariable “position p” here? Regards, Markus
Re: [PATCH v2 1/2] treewide: remove unused address argument from pte_alloc functions
>>> The changes were obtained by applying the following Coccinelle script. How do you think about to adjust the order of provided information in the commit description? 1. Update goals 2. Transformation implementation at the end >> "^(?:pte_alloc(?:_one(?:_kernel)?)?|__pte_alloc(?:_kernel)?)$"; > > Sure it looks more clever, but why? 1. Usage of non-capturing parentheses 2. Clearer specification which parts can be treated as optional in the search pattern. > Ugh that's harder to read and confusing. * Do you care for coding style and execution speed of regular expressions? * If you would prefer to list function names without placeholders, you can eventually specify them also within SmPL disjunctions directly. * It can look simpler to use an identifier list as a constraint variant. http://coccinelle.lip6.fr/docs/main_grammar002.html > Again this is confusing. The view points can be different for such SmPL code. T3 fn(T1 E1 ( - , T2 E2 | , T2 E2 - , T4 E4 ) ); > It makes one think that maybe the second argument can also be removed You expressed this as the first transformation possibility, didn't you? You would like to delete an argument from the end of a function or macro parameter (or expression) list. I suggest then again to avoid the SmPL specification of source code additions (plus lines in the file difference format). > and requires careful observation that the ");" follows. Yes, of course. Would you care more in the distinction which code parts should be kept unchanged? > Right, I don't need it in this case. Thanks for your understanding that the metavariable “position p” can be deleted in the SmPL rule “pte_alloc_macro”. > But the script works either way. I imagine that you can become interested in a bit nicer run time characteristics. > I like to take more of a problem solving approach that makes sense, This is usual. > than aiming for perfection, If you will work more with scripts for the semantic patch language, you might become used to additional coding variants. > after all this is a useful script that we do not need to check > in once we finish with it. I am curious if there will evolve a need to add similar transformation approaches to a known script collection. https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/scripts/coccinelle?id=79fc170b1f5c36f486d886bfbd59eb4e62321128 Would you eventually like to run such scripts once more? Regards, Markus
Re: [PATCH -next v2 1/3] mm: treewide: remove unused address argument from pte_alloc functions
> … There is concern that … Does this wording need a small adjustment? > The changes were obtained by applying the following Coccinelle script. I would find it nicer if previous patch review comments will trigger further useful effects here. https://patchwork.kernel.org/patch/10637703/#22265203 https://lore.kernel.org/linuxppc-dev/03b524f3-5f3a-baa0-2254-9c588103d...@users.sourceforge.net/ https://www.mail-archive.com/linuxppc-dev@lists.ozlabs.org/msg140009.html If you have got difficulties with the usage of advanced regular expressions for SmPL constraints, I suggest to use desired function names in SmPL lists or disjunctions instead because of different run time characteristics for such a source code transformation approach. > // Note: I split the 'identifier fn' line, so if you are manually > // running it, please unsplit it so it runs for you. Please delete this questionable comment. * The semantic patch language should handle the mentioned code formatting. * You can use multi-line regular expressions (if it would be desired). > @pte_alloc_func_def depends on patch exists@ > identifier E2; > identifier fn =~ How do you think about to avoid the repetition of a SmPL key word at such places? Regards, Markus
[PATCH] powerpc/mm: Use seq_putc() in two functions
From: Markus Elfring Date: Sun, 7 May 2017 16:32:04 +0200 Two single characters (line breaks) should be put into a sequence. Thus use the corresponding function "seq_putc". This issue was detected by using the Coccinelle software. Signed-off-by: Markus Elfring --- arch/powerpc/mm/dump_hashpagetable.c | 2 +- arch/powerpc/mm/dump_linuxpagetables.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/arch/powerpc/mm/dump_hashpagetable.c b/arch/powerpc/mm/dump_hashpagetable.c index c6b900f54c07..31d248c82f95 100644 --- a/arch/powerpc/mm/dump_hashpagetable.c +++ b/arch/powerpc/mm/dump_hashpagetable.c @@ -205,7 +205,7 @@ static void dump_hpte_info(struct pg_state *st, unsigned long ea, u64 v, u64 r, aps_index = calculate_pagesize(st, aps, "actual"); if (aps_index != 2) seq_printf(st->seq, "LP enc: %lx", lp); - seq_puts(st->seq, "\n"); + seq_putc(st->seq, '\n'); } diff --git a/arch/powerpc/mm/dump_linuxpagetables.c b/arch/powerpc/mm/dump_linuxpagetables.c index d659345a98d6..5c08de16339d 100644 --- a/arch/powerpc/mm/dump_linuxpagetables.c +++ b/arch/powerpc/mm/dump_linuxpagetables.c @@ -349,7 +349,7 @@ static void note_page(struct pg_state *st, unsigned long addr, st->current_flags, pg_level[st->level].num); - seq_puts(st->seq, "\n"); + seq_putc(st->seq, '\n'); } /* -- 2.12.2
[PATCH] KVM: PPC: Book3S HV: Delete an error message for a failed memory allocation in kvmppc_allocate_hpt()
From: Markus Elfring
Date: Thu, 5 Oct 2017 13:16:51 +0200
Omit an extra message for a memory allocation failure in this function.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring
---
arch/powerpc/kvm/book3s_64_mmu_hv.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/arch/powerpc/kvm/book3s_64_mmu_hv.c
b/arch/powerpc/kvm/book3s_64_mmu_hv.c
index 7c62967d672c..7df89d58ee91 100644
--- a/arch/powerpc/kvm/book3s_64_mmu_hv.c
+++ b/arch/powerpc/kvm/book3s_64_mmu_hv.c
@@ -106,7 +106,6 @@ int kvmppc_allocate_hpt(struct kvm_hpt_info *info, u32
order)
/* Allocate reverse map array */
rev = vmalloc(sizeof(struct revmap_entry) * npte);
if (!rev) {
- pr_err("kvmppc_allocate_hpt: Couldn't alloc reverse map
array\n");
if (cma)
kvm_free_hpt_cma(page, 1 << (order - PAGE_SHIFT));
else
--
2.14.2
[PATCH 0/2] PowerPC-Cell OProfile: Adjustments for three function implementations
From: Markus Elfring Date: Thu, 5 Oct 2017 17:32:10 +0200 Two update suggestions were taken into account from static source code analysis. Markus Elfring (2): Delete an error message for a failed memory allocation in three functions Improve a size determination in two functions arch/powerpc/oprofile/cell/spu_task_sync.c | 10 +- arch/powerpc/oprofile/cell/vma_map.c | 6 ++ 2 files changed, 3 insertions(+), 13 deletions(-) -- 2.14.2
[PATCH 1/2] powerpc/oprofile/cell: Delete an error message for a failed memory allocation in three functions
From: Markus Elfring
Date: Thu, 5 Oct 2017 17:10:11 +0200
Omit extra messages for a memory allocation failure in these functions.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring
---
arch/powerpc/oprofile/cell/spu_task_sync.c | 8
arch/powerpc/oprofile/cell/vma_map.c | 2 --
2 files changed, 10 deletions(-)
diff --git a/arch/powerpc/oprofile/cell/spu_task_sync.c
b/arch/powerpc/oprofile/cell/spu_task_sync.c
index 44d67b167e0b..f1dd732b2998 100644
--- a/arch/powerpc/oprofile/cell/spu_task_sync.c
+++ b/arch/powerpc/oprofile/cell/spu_task_sync.c
@@ -210,9 +210,6 @@ prepare_cached_spu_info(struct spu *spu, unsigned long
objectId)
*/
info = kzalloc(sizeof(struct cached_info), GFP_KERNEL);
if (!info) {
- printk(KERN_ERR "SPU_PROF: "
- "%s, line %d: create vma_map failed\n",
- __func__, __LINE__);
retval = -ENOMEM;
goto err_alloc;
}
@@ -481,11 +478,6 @@ static int oprofile_spu_buff_create(void)
GFP_KERNEL);
if (!spu_buff[spu].buff) {
- printk(KERN_ERR "SPU_PROF: "
- "%s, line %d: oprofile_spu_buff_create "
- "failed to allocate spu buffer %d.\n",
- __func__, __LINE__, spu);
-
/* release the spu buffers that have been allocated */
while (spu >= 0) {
kfree(spu_buff[spu].buff);
diff --git a/arch/powerpc/oprofile/cell/vma_map.c
b/arch/powerpc/oprofile/cell/vma_map.c
index c579b16845da..a115d9ede053 100644
--- a/arch/powerpc/oprofile/cell/vma_map.c
+++ b/arch/powerpc/oprofile/cell/vma_map.c
@@ -72,8 +72,6 @@ vma_map_add(struct vma_to_fileoffset_map *map, unsigned int
vma,
struct vma_to_fileoffset_map *new =
kzalloc(sizeof(struct vma_to_fileoffset_map), GFP_KERNEL);
if (!new) {
- printk(KERN_ERR "SPU_PROF: %s, line %d: malloc failed\n",
- __func__, __LINE__);
vma_map_free(map);
return NULL;
}
--
2.14.2
[PATCH 2/2] powerpc/oprofile/cell: Improve a size determination in two functions
From: Markus Elfring
Date: Thu, 5 Oct 2017 17:18:33 +0200
Replace the specification of data structures by pointer dereferences
as the parameter for the operator "sizeof" to make the corresponding size
determination a bit safer according to the Linux coding style convention.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring
---
arch/powerpc/oprofile/cell/spu_task_sync.c | 2 +-
arch/powerpc/oprofile/cell/vma_map.c | 4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/oprofile/cell/spu_task_sync.c
b/arch/powerpc/oprofile/cell/spu_task_sync.c
index f1dd732b2998..8ab442925296 100644
--- a/arch/powerpc/oprofile/cell/spu_task_sync.c
+++ b/arch/powerpc/oprofile/cell/spu_task_sync.c
@@ -208,7 +208,7 @@ prepare_cached_spu_info(struct spu *spu, unsigned long
objectId)
/* Create cached_info and set spu_info[spu->number] to point to it.
* spu->number is a system-wide value, not a per-node value.
*/
- info = kzalloc(sizeof(struct cached_info), GFP_KERNEL);
+ info = kzalloc(sizeof(*info), GFP_KERNEL);
if (!info) {
retval = -ENOMEM;
goto err_alloc;
diff --git a/arch/powerpc/oprofile/cell/vma_map.c
b/arch/powerpc/oprofile/cell/vma_map.c
index a115d9ede053..03de415a377c 100644
--- a/arch/powerpc/oprofile/cell/vma_map.c
+++ b/arch/powerpc/oprofile/cell/vma_map.c
@@ -69,8 +69,8 @@ vma_map_add(struct vma_to_fileoffset_map *map, unsigned int
vma,
unsigned int size, unsigned int offset, unsigned int guard_ptr,
unsigned int guard_val)
{
- struct vma_to_fileoffset_map *new =
- kzalloc(sizeof(struct vma_to_fileoffset_map), GFP_KERNEL);
+ struct vma_to_fileoffset_map *new = kzalloc(sizeof(*new), GFP_KERNEL);
+
if (!new) {
vma_map_free(map);
return NULL;
--
2.14.2
[PATCH] powerpc/perf/hv-24x7: Delete an error message for a failed memory allocation in create_events_from_catalog()
From: Markus Elfring
Date: Thu, 5 Oct 2017 18:02:05 +0200
Omit an extra message for a memory allocation failure in this function.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring
---
arch/powerpc/perf/hv-24x7.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/arch/powerpc/perf/hv-24x7.c b/arch/powerpc/perf/hv-24x7.c
index 9c88b82f6229..f21f30aaf9bd 100644
--- a/arch/powerpc/perf/hv-24x7.c
+++ b/arch/powerpc/perf/hv-24x7.c
@@ -766,7 +766,6 @@ static int create_events_from_catalog(struct attribute
***events_,
*/
event_data = vmalloc(event_data_bytes);
if (!event_data) {
- pr_err("could not allocate event data\n");
ret = -ENOMEM;
goto e_free;
}
--
2.14.2
[PATCH 0/2] PowerPC-Cell platform: Adjustments for four function implementations
From: Markus Elfring Date: Thu, 5 Oct 2017 21:25:43 +0200 Two update suggestions were taken into account from static source code analysis. Markus Elfring (2): Delete an error message for a failed memory allocation in three functions Improve a size determination in three functions arch/powerpc/platforms/cell/axon_msi.c | 7 ++- arch/powerpc/platforms/cell/spider-pci.c| 11 +++ arch/powerpc/platforms/cell/spufs/lscsa_alloc.c | 2 +- 3 files changed, 6 insertions(+), 14 deletions(-) -- 2.14.2
[PATCH 1/2] powerpc/platforms/cell: Delete an error message for a failed memory allocation in three functions
From: Markus Elfring
Date: Thu, 5 Oct 2017 21:04:30 +0200
Omit extra messages for a memory allocation failure in these functions.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring
---
arch/powerpc/platforms/cell/axon_msi.c | 5 +
arch/powerpc/platforms/cell/spider-pci.c | 9 ++---
2 files changed, 3 insertions(+), 11 deletions(-)
diff --git a/arch/powerpc/platforms/cell/axon_msi.c
b/arch/powerpc/platforms/cell/axon_msi.c
index 6ea3f248b155..513d13d779b3 100644
--- a/arch/powerpc/platforms/cell/axon_msi.c
+++ b/arch/powerpc/platforms/cell/axon_msi.c
@@ -343,11 +343,8 @@ static int axon_msi_probe(struct platform_device *device)
pr_devel("axon_msi: setting up dn %pOF\n", dn);
msic = kzalloc(sizeof(struct axon_msic), GFP_KERNEL);
- if (!msic) {
- printk(KERN_ERR "axon_msi: couldn't allocate msic for %pOF\n",
- dn);
+ if (!msic)
goto out;
- }
dcr_base = dcr_resource_start(dn, 0);
dcr_len = dcr_resource_len(dn, 0);
diff --git a/arch/powerpc/platforms/cell/spider-pci.c
b/arch/powerpc/platforms/cell/spider-pci.c
index d1e61e273e64..0a9f00563144 100644
--- a/arch/powerpc/platforms/cell/spider-pci.c
+++ b/arch/powerpc/platforms/cell/spider-pci.c
@@ -104,10 +104,8 @@ static int __init spiderpci_pci_setup_chip(struct
pci_controller *phb,
* Celleb does not have this problem, because it has only one XDR.
*/
dummy_page_va = kmalloc(PAGE_SIZE, GFP_KERNEL);
- if (!dummy_page_va) {
- pr_err("SPIDERPCI-IOWA:Alloc dummy_page_va failed.\n");
+ if (!dummy_page_va)
return -1;
- }
dummy_page_da = dma_map_single(phb->parent, dummy_page_va,
PAGE_SIZE, DMA_FROM_DEVICE);
@@ -134,11 +132,8 @@ int __init spiderpci_iowa_init(struct iowa_bus *bus, void
*data)
np);
priv = kzalloc(sizeof(struct spiderpci_iowa_private), GFP_KERNEL);
- if (!priv) {
- pr_err("SPIDERPCI-IOWA:"
- "Can't allocate struct spiderpci_iowa_private");
+ if (!priv)
return -1;
- }
if (of_address_to_resource(np, 0, &r)) {
pr_err("SPIDERPCI-IOWA:Can't get resource.\n");
--
2.14.2
[PATCH 2/2] powerpc/platforms/cell: Improve a size determination in three functions
From: Markus Elfring
Date: Thu, 5 Oct 2017 21:12:41 +0200
Replace the specification of data structures by pointer dereferences
as the parameter for the operator "sizeof" to make the corresponding size
determination a bit safer according to the Linux coding style convention.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring
---
arch/powerpc/platforms/cell/axon_msi.c | 2 +-
arch/powerpc/platforms/cell/spider-pci.c| 2 +-
arch/powerpc/platforms/cell/spufs/lscsa_alloc.c | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/platforms/cell/axon_msi.c
b/arch/powerpc/platforms/cell/axon_msi.c
index 513d13d779b3..cc4987664841 100644
--- a/arch/powerpc/platforms/cell/axon_msi.c
+++ b/arch/powerpc/platforms/cell/axon_msi.c
@@ -342,7 +342,7 @@ static int axon_msi_probe(struct platform_device *device)
pr_devel("axon_msi: setting up dn %pOF\n", dn);
- msic = kzalloc(sizeof(struct axon_msic), GFP_KERNEL);
+ msic = kzalloc(sizeof(*msic), GFP_KERNEL);
if (!msic)
goto out;
diff --git a/arch/powerpc/platforms/cell/spider-pci.c
b/arch/powerpc/platforms/cell/spider-pci.c
index 0a9f00563144..31919ff00124 100644
--- a/arch/powerpc/platforms/cell/spider-pci.c
+++ b/arch/powerpc/platforms/cell/spider-pci.c
@@ -131,7 +131,7 @@ int __init spiderpci_iowa_init(struct iowa_bus *bus, void
*data)
pr_debug("SPIDERPCI-IOWA:Bus initialize for spider(%pOF)\n",
np);
- priv = kzalloc(sizeof(struct spiderpci_iowa_private), GFP_KERNEL);
+ priv = kzalloc(sizeof(*priv), GFP_KERNEL);
if (!priv)
return -1;
diff --git a/arch/powerpc/platforms/cell/spufs/lscsa_alloc.c
b/arch/powerpc/platforms/cell/spufs/lscsa_alloc.c
index b847e9403566..d9de848dae47 100644
--- a/arch/powerpc/platforms/cell/spufs/lscsa_alloc.c
+++ b/arch/powerpc/platforms/cell/spufs/lscsa_alloc.c
@@ -36,7 +36,7 @@ int spu_alloc_lscsa(struct spu_state *csa)
struct spu_lscsa *lscsa;
unsigned char *p;
- lscsa = vzalloc(sizeof(struct spu_lscsa));
+ lscsa = vzalloc(sizeof(*lscsa));
if (!lscsa)
return -ENOMEM;
csa->lscsa = lscsa;
--
2.14.2
[PATCH 0/2] Power Mac: Adjustments for five function implementations
From: Markus Elfring Date: Thu, 5 Oct 2017 22:48:22 +0200 Two update suggestions were taken into account from static source code analysis. Markus Elfring (2): Delete an error message for a failed memory allocation in kw_i2c_host_init() Improve a size determination in five functions arch/powerpc/platforms/powermac/low_i2c.c| 11 --- arch/powerpc/platforms/powermac/pfunc_core.c | 4 ++-- 2 files changed, 6 insertions(+), 9 deletions(-) -- 2.14.2
[PATCH 1/2] powermac: Delete an error message for a failed memory allocation in kw_i2c_host_init()
From: Markus Elfring
Date: Thu, 5 Oct 2017 22:30:29 +0200
Omit an extra message for a memory allocation failure in this function.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring
---
arch/powerpc/platforms/powermac/low_i2c.c | 5 +
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/arch/powerpc/platforms/powermac/low_i2c.c
b/arch/powerpc/platforms/powermac/low_i2c.c
index 70183eb3d5c8..ecd166b5549b 100644
--- a/arch/powerpc/platforms/powermac/low_i2c.c
+++ b/arch/powerpc/platforms/powermac/low_i2c.c
@@ -493,11 +493,8 @@ static struct pmac_i2c_host_kw *__init
kw_i2c_host_init(struct device_node *np)
u32 steps;
host = kzalloc(sizeof(struct pmac_i2c_host_kw), GFP_KERNEL);
- if (host == NULL) {
- printk(KERN_ERR "low_i2c: Can't allocate host for %pOF\n",
- np);
+ if (!host)
return NULL;
- }
/* Apple is kind enough to provide a valid AAPL,address property
* on all i2c keywest nodes so far ... we would have to fallback
--
2.14.2
[PATCH 2/2] powermac: Improve a size determination in five functions
From: Markus Elfring
Date: Thu, 5 Oct 2017 22:40:39 +0200
Replace the specification of data structures by pointer dereferences
as the parameter for the operator "sizeof" to make the corresponding size
determination a bit safer according to the Linux coding style convention.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring
---
arch/powerpc/platforms/powermac/low_i2c.c| 6 +++---
arch/powerpc/platforms/powermac/pfunc_core.c | 4 ++--
2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/platforms/powermac/low_i2c.c
b/arch/powerpc/platforms/powermac/low_i2c.c
index ecd166b5549b..46b7eb1a0f5f 100644
--- a/arch/powerpc/platforms/powermac/low_i2c.c
+++ b/arch/powerpc/platforms/powermac/low_i2c.c
@@ -492,7 +492,7 @@ static struct pmac_i2c_host_kw *__init
kw_i2c_host_init(struct device_node *np)
const u32 *psteps, *prate, *addrp;
u32 steps;
- host = kzalloc(sizeof(struct pmac_i2c_host_kw), GFP_KERNEL);
+ host = kzalloc(sizeof(*host), GFP_KERNEL);
if (!host)
return NULL;
@@ -571,7 +571,7 @@ static void __init kw_i2c_add(struct pmac_i2c_host_kw *host,
{
struct pmac_i2c_bus *bus;
- bus = kzalloc(sizeof(struct pmac_i2c_bus), GFP_KERNEL);
+ bus = kzalloc(sizeof(*bus), GFP_KERNEL);
if (bus == NULL)
return;
@@ -1253,7 +1253,7 @@ static void* pmac_i2c_do_begin(struct pmf_function *func,
struct pmf_args *args)
* near OOM that need to be resolved, the allocator itself should
* probably make GFP_NOIO implicit during suspend
*/
- inst = kzalloc(sizeof(struct pmac_i2c_pf_inst), GFP_KERNEL);
+ inst = kzalloc(sizeof(*inst), GFP_KERNEL);
if (inst == NULL) {
pmac_i2c_close(bus);
return NULL;
diff --git a/arch/powerpc/platforms/powermac/pfunc_core.c
b/arch/powerpc/platforms/powermac/pfunc_core.c
index df3c93bef228..e0462fedcdb8 100644
--- a/arch/powerpc/platforms/powermac/pfunc_core.c
+++ b/arch/powerpc/platforms/powermac/pfunc_core.c
@@ -643,7 +643,7 @@ static int pmf_add_function_prop(struct pmf_device *dev,
void *driverdata,
while (length >= 12) {
/* Allocate a structure */
- func = kzalloc(sizeof(struct pmf_function), GFP_KERNEL);
+ func = kzalloc(sizeof(*func), GFP_KERNEL);
if (func == NULL)
goto bail;
kref_init(&func->ref);
@@ -719,7 +719,7 @@ int pmf_register_driver(struct device_node *np,
return -EBUSY;
}
- dev = kzalloc(sizeof(struct pmf_device), GFP_KERNEL);
+ dev = kzalloc(sizeof(*dev), GFP_KERNEL);
if (dev == NULL) {
DBG("pmf: no memory !\n");
return -ENOMEM;
--
2.14.2
[PATCH 0/4] char-TPM: Adjustments for ten function implementations
From: Markus Elfring Date: Mon, 16 Oct 2017 19:12:34 +0200 A few update suggestions were taken into account from static source code analysis. Markus Elfring (4): Delete an error message for a failed memory allocation in tpm_ascii_bios_measurements_show() Delete an error message for a failed memory allocation in tpm_ibmvtpm_probe() Improve a size determination in nine functions Less checks in tpm_ibmvtpm_probe() after error detection drivers/char/tpm/st33zp24/i2c.c | 3 +-- drivers/char/tpm/st33zp24/spi.c | 3 +-- drivers/char/tpm/st33zp24/st33zp24.c | 3 +-- drivers/char/tpm/tpm1_eventlog.c | 5 + drivers/char/tpm/tpm_crb.c | 2 +- drivers/char/tpm/tpm_i2c_atmel.c | 2 +- drivers/char/tpm/tpm_i2c_nuvoton.c | 2 +- drivers/char/tpm/tpm_ibmvtpm.c | 23 +-- drivers/char/tpm/tpm_tis.c | 2 +- drivers/char/tpm/tpm_tis_spi.c | 3 +-- 10 files changed, 18 insertions(+), 30 deletions(-) -- 2.14.2
[PATCH 1/4] char/tpm: Delete an error message for a failed memory allocation in tpm_ascii_bios_measurements_show()
From: Markus Elfring
Date: Mon, 16 Oct 2017 17:43:55 +0200
Omit an extra message for a memory allocation failure in this function.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring
---
drivers/char/tpm/tpm1_eventlog.c | 5 +
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/drivers/char/tpm/tpm1_eventlog.c b/drivers/char/tpm/tpm1_eventlog.c
index 9a8605e500b5..9749469cb823 100644
--- a/drivers/char/tpm/tpm1_eventlog.c
+++ b/drivers/char/tpm/tpm1_eventlog.c
@@ -280,11 +280,8 @@ static int tpm_ascii_bios_measurements_show(struct
seq_file *m, void *v)
(unsigned char *)(v + sizeof(struct tcpa_event));
eventname = kmalloc(MAX_TEXT_EVENT, GFP_KERNEL);
- if (!eventname) {
- printk(KERN_ERR "%s: ERROR - No Memory for event name\n ",
- __func__);
+ if (!eventname)
return -EFAULT;
- }
/* 1st: PCR */
seq_printf(m, "%2d ", do_endian_conversion(event->pcr_index));
--
2.14.2
[PATCH 2/4] char/tpm: Delete an error message for a failed memory allocation in tpm_ibmvtpm_probe()
From: Markus Elfring
Date: Mon, 16 Oct 2017 18:08:23 +0200
Omit an extra message for a memory allocation failure in this function.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring
---
drivers/char/tpm/tpm_ibmvtpm.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/char/tpm/tpm_ibmvtpm.c b/drivers/char/tpm/tpm_ibmvtpm.c
index 25f6e2665385..b18148ef2612 100644
--- a/drivers/char/tpm/tpm_ibmvtpm.c
+++ b/drivers/char/tpm/tpm_ibmvtpm.c
@@ -609,10 +609,8 @@ static int tpm_ibmvtpm_probe(struct vio_dev *vio_dev,
return PTR_ERR(chip);
ibmvtpm = kzalloc(sizeof(struct ibmvtpm_dev), GFP_KERNEL);
- if (!ibmvtpm) {
- dev_err(dev, "kzalloc for ibmvtpm failed\n");
+ if (!ibmvtpm)
goto cleanup;
- }
ibmvtpm->dev = dev;
ibmvtpm->vdev = vio_dev;
--
2.14.2
[PATCH 3/4] char/tpm: Improve a size determination in nine functions
From: Markus Elfring Date: Mon, 16 Oct 2017 18:28:17 +0200 Replace the specification of data structures by pointer dereferences as the parameter for the operator "sizeof" to make the corresponding size determination a bit safer according to the Linux coding style convention. This issue was detected by using the Coccinelle software. Signed-off-by: Markus Elfring --- drivers/char/tpm/st33zp24/i2c.c | 3 +-- drivers/char/tpm/st33zp24/spi.c | 3 +-- drivers/char/tpm/st33zp24/st33zp24.c | 3 +-- drivers/char/tpm/tpm_crb.c | 2 +- drivers/char/tpm/tpm_i2c_atmel.c | 2 +- drivers/char/tpm/tpm_i2c_nuvoton.c | 2 +- drivers/char/tpm/tpm_ibmvtpm.c | 2 +- drivers/char/tpm/tpm_tis.c | 2 +- drivers/char/tpm/tpm_tis_spi.c | 3 +-- 9 files changed, 9 insertions(+), 13 deletions(-) diff --git a/drivers/char/tpm/st33zp24/i2c.c b/drivers/char/tpm/st33zp24/i2c.c index be5d1abd3e8e..d0cb25688485 100644 --- a/drivers/char/tpm/st33zp24/i2c.c +++ b/drivers/char/tpm/st33zp24/i2c.c @@ -245,8 +245,7 @@ static int st33zp24_i2c_probe(struct i2c_client *client, return -ENODEV; } - phy = devm_kzalloc(&client->dev, sizeof(struct st33zp24_i2c_phy), - GFP_KERNEL); + phy = devm_kzalloc(&client->dev, sizeof(*phy), GFP_KERNEL); if (!phy) return -ENOMEM; diff --git a/drivers/char/tpm/st33zp24/spi.c b/drivers/char/tpm/st33zp24/spi.c index 0fc4f20b5f83..c952df9244c8 100644 --- a/drivers/char/tpm/st33zp24/spi.c +++ b/drivers/char/tpm/st33zp24/spi.c @@ -358,8 +358,7 @@ static int st33zp24_spi_probe(struct spi_device *dev) return -ENODEV; } - phy = devm_kzalloc(&dev->dev, sizeof(struct st33zp24_spi_phy), - GFP_KERNEL); + phy = devm_kzalloc(&dev->dev, sizeof(*phy), GFP_KERNEL); if (!phy) return -ENOMEM; diff --git a/drivers/char/tpm/st33zp24/st33zp24.c b/drivers/char/tpm/st33zp24/st33zp24.c index 4d1dc8b46877..0686a129268c 100644 --- a/drivers/char/tpm/st33zp24/st33zp24.c +++ b/drivers/char/tpm/st33zp24/st33zp24.c @@ -533,8 +533,7 @@ int st33zp24_probe(void *phy_id, const struct st33zp24_phy_ops *ops, if (IS_ERR(chip)) return PTR_ERR(chip); - tpm_dev = devm_kzalloc(dev, sizeof(struct st33zp24_dev), - GFP_KERNEL); + tpm_dev = devm_kzalloc(dev, sizeof(*tpm_dev), GFP_KERNEL); if (!tpm_dev) return -ENOMEM; diff --git a/drivers/char/tpm/tpm_crb.c b/drivers/char/tpm/tpm_crb.c index 7b3c2a8aa9de..343c46e8560f 100644 --- a/drivers/char/tpm/tpm_crb.c +++ b/drivers/char/tpm/tpm_crb.c @@ -557,7 +557,7 @@ static int crb_acpi_add(struct acpi_device *device) if (sm == ACPI_TPM2_MEMORY_MAPPED) return -ENODEV; - priv = devm_kzalloc(dev, sizeof(struct crb_priv), GFP_KERNEL); + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL); if (!priv) return -ENOMEM; diff --git a/drivers/char/tpm/tpm_i2c_atmel.c b/drivers/char/tpm/tpm_i2c_atmel.c index 95ce2e9ccdc6..2d0df930a76d 100644 --- a/drivers/char/tpm/tpm_i2c_atmel.c +++ b/drivers/char/tpm/tpm_i2c_atmel.c @@ -165,7 +165,7 @@ static int i2c_atmel_probe(struct i2c_client *client, if (IS_ERR(chip)) return PTR_ERR(chip); - priv = devm_kzalloc(dev, sizeof(struct priv_data), GFP_KERNEL); + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL); if (!priv) return -ENOMEM; diff --git a/drivers/char/tpm/tpm_i2c_nuvoton.c b/drivers/char/tpm/tpm_i2c_nuvoton.c index c6428771841f..5983d52eb6d9 100644 --- a/drivers/char/tpm/tpm_i2c_nuvoton.c +++ b/drivers/char/tpm/tpm_i2c_nuvoton.c @@ -551,7 +551,7 @@ static int i2c_nuvoton_probe(struct i2c_client *client, if (IS_ERR(chip)) return PTR_ERR(chip); - priv = devm_kzalloc(dev, sizeof(struct priv_data), GFP_KERNEL); + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL); if (!priv) return -ENOMEM; diff --git a/drivers/char/tpm/tpm_ibmvtpm.c b/drivers/char/tpm/tpm_ibmvtpm.c index b18148ef2612..a4b462a77b99 100644 --- a/drivers/char/tpm/tpm_ibmvtpm.c +++ b/drivers/char/tpm/tpm_ibmvtpm.c @@ -608,7 +608,7 @@ static int tpm_ibmvtpm_probe(struct vio_dev *vio_dev, if (IS_ERR(chip)) return PTR_ERR(chip); - ibmvtpm = kzalloc(sizeof(struct ibmvtpm_dev), GFP_KERNEL); + ibmvtpm = kzalloc(sizeof(*ibmvtpm), GFP_KERNEL); if (!ibmvtpm) goto cleanup; diff --git a/drivers/char/tpm/tpm_tis.c b/drivers/char/tpm/tpm_tis.c index ebd0e75a3e4d..0a3af60bab2a 100644 --- a/drivers/char/tpm/tpm_tis.c +++ b/drivers/char/tpm/tpm_tis.c @@ -294,7 +294,7 @@ static int tpm_tis_init(struct device *dev, struct tpm_info *tpm_info) if (rc) return rc; - phy = devm_kzalloc(dev, sizeof(struct tpm_tis_tcg_phy), GFP_KERNEL); +
[PATCH 4/4] char/tpm: Less checks in tpm_ibmvtpm_probe() after error detection
From: Markus Elfring
Date: Mon, 16 Oct 2017 19:00:34 +0200
Two pointer checks could be repeated by the tpm_ibmvtpm_probe() function
during error handling even if the relevant properties can be determined
for the involved variables before by source code analysis.
* Return directly after a call of the function "kzalloc" failed
at the beginning.
* Adjust jump targets so that extra checks can be omitted at the end.
Signed-off-by: Markus Elfring
---
drivers/char/tpm/tpm_ibmvtpm.c | 17 +++--
1 file changed, 7 insertions(+), 10 deletions(-)
diff --git a/drivers/char/tpm/tpm_ibmvtpm.c b/drivers/char/tpm/tpm_ibmvtpm.c
index a4b462a77b99..b8dda7546f64 100644
--- a/drivers/char/tpm/tpm_ibmvtpm.c
+++ b/drivers/char/tpm/tpm_ibmvtpm.c
@@ -610,7 +610,7 @@ static int tpm_ibmvtpm_probe(struct vio_dev *vio_dev,
ibmvtpm = kzalloc(sizeof(*ibmvtpm), GFP_KERNEL);
if (!ibmvtpm)
- goto cleanup;
+ return -ENOMEM;
ibmvtpm->dev = dev;
ibmvtpm->vdev = vio_dev;
@@ -619,7 +619,7 @@ static int tpm_ibmvtpm_probe(struct vio_dev *vio_dev,
crq_q->crq_addr = (struct ibmvtpm_crq *)get_zeroed_page(GFP_KERNEL);
if (!crq_q->crq_addr) {
dev_err(dev, "Unable to allocate memory for crq_addr\n");
- goto cleanup;
+ goto free_tpm;
}
crq_q->num_entry = CRQ_RES_BUF_SIZE / sizeof(*crq_q->crq_addr);
@@ -629,7 +629,7 @@ static int tpm_ibmvtpm_probe(struct vio_dev *vio_dev,
if (dma_mapping_error(dev, ibmvtpm->crq_dma_handle)) {
dev_err(dev, "dma mapping failed\n");
- goto cleanup;
+ goto free_page;
}
rc = plpar_hcall_norets(H_REG_CRQ, vio_dev->unit_address,
@@ -683,13 +683,10 @@ static int tpm_ibmvtpm_probe(struct vio_dev *vio_dev,
reg_crq_cleanup:
dma_unmap_single(dev, ibmvtpm->crq_dma_handle, CRQ_RES_BUF_SIZE,
DMA_BIDIRECTIONAL);
-cleanup:
- if (ibmvtpm) {
- if (crq_q->crq_addr)
- free_page((unsigned long)crq_q->crq_addr);
- kfree(ibmvtpm);
- }
-
+free_page:
+ free_page((unsigned long)crq_q->crq_addr);
+free_tpm:
+ kfree(ibmvtpm);
return rc;
}
--
2.14.2
Re: [PATCH 0/4] char-TPM: Adjustments for ten function implementations
> A minor complaint: all commits are missing "Fixes:" tag. * Do you require it to be added to the commit messages? * Would you like to get a finer patch granularity then? * Do you find any more information missing? Regards, Markus
Re: [PATCH 0/4] char-TPM: Adjustments for ten function implementations
>> A minor complaint: all commits are missing "Fixes:" tag. > > None of these patches fix anything. It depends on the view which you prefer. > All are trivial changes without much of any impact. I find that they improve the affected software another bit. Other adjustments can be more noticeable. Regards, Markus
Re: [PATCH 0/4] char-TPM: Adjustments for ten function implementations
> Fixes is only for bug fixes. These don't fix any bugs. How do you distinguish these in questionable source code from other error categories or software weaknesses? Regards, Markus
[PATCH 0/3] PowerPC-OPAL: Adjustments for some function implementations
From: Markus Elfring Date: Tue, 17 Oct 2017 13:45:54 +0200 Three update suggestions were taken into account from static source code analysis. Markus Elfring (3): Delete ten error messages for a failed memory allocation Improve 12 size determinations Fix a typo in a comment line of two file headers arch/powerpc/platforms/powernv/opal-async.c| 2 -- arch/powerpc/platforms/powernv/opal-dump.c | 1 - arch/powerpc/platforms/powernv/opal-flash.c| 8 +++ arch/powerpc/platforms/powernv/opal-hmi.c | 9 arch/powerpc/platforms/powernv/opal-imc.c | 10 - .../powerpc/platforms/powernv/opal-memory-errors.c | 10 - arch/powerpc/platforms/powernv/opal-psr.c | 2 +- .../powerpc/platforms/powernv/opal-sensor-groups.c | 4 ++-- arch/powerpc/platforms/powernv/opal-sysparam.c | 25 +- arch/powerpc/platforms/powernv/opal-xscom.c| 2 +- 10 files changed, 25 insertions(+), 48 deletions(-) -- 2.14.2
[PATCH 1/3] powerpc-opal: Delete ten error messages for a failed memory allocation
From: Markus Elfring
Date: Tue, 17 Oct 2017 12:21:40 +0200
Omit extra messages for a memory allocation failure in these functions.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring
---
arch/powerpc/platforms/powernv/opal-async.c| 2 --
arch/powerpc/platforms/powernv/opal-dump.c | 1 -
arch/powerpc/platforms/powernv/opal-flash.c| 4 +---
arch/powerpc/platforms/powernv/opal-hmi.c | 5 ++---
.../powerpc/platforms/powernv/opal-memory-errors.c | 6 ++
arch/powerpc/platforms/powernv/opal-sysparam.c | 25 +-
6 files changed, 10 insertions(+), 33 deletions(-)
diff --git a/arch/powerpc/platforms/powernv/opal-async.c
b/arch/powerpc/platforms/powernv/opal-async.c
index cf33769a7b72..2b11d8444b5a 100644
--- a/arch/powerpc/platforms/powernv/opal-async.c
+++ b/arch/powerpc/platforms/powernv/opal-async.c
@@ -193,8 +193,6 @@ int __init opal_async_comp_init(void)
sizeof(*opal_async_responses) * opal_max_async_tokens,
GFP_KERNEL);
if (!opal_async_responses) {
- pr_err("%s: Out of memory, failed to do asynchronous "
- "completion init\n", __func__);
err = -ENOMEM;
goto out_opal_node;
}
diff --git a/arch/powerpc/platforms/powernv/opal-dump.c
b/arch/powerpc/platforms/powernv/opal-dump.c
index 4c827826c05e..b54864feef84 100644
--- a/arch/powerpc/platforms/powernv/opal-dump.c
+++ b/arch/powerpc/platforms/powernv/opal-dump.c
@@ -244,7 +244,6 @@ static int64_t dump_read_data(struct dump_obj *dump)
/* Allocate memory */
dump->buffer = vzalloc(PAGE_ALIGN(dump->size));
if (!dump->buffer) {
- pr_err("%s : Failed to allocate memory\n", __func__);
rc = -ENOMEM;
goto out;
}
diff --git a/arch/powerpc/platforms/powernv/opal-flash.c
b/arch/powerpc/platforms/powernv/opal-flash.c
index 2fa3ac80cb4e..77d564266a59 100644
--- a/arch/powerpc/platforms/powernv/opal-flash.c
+++ b/arch/powerpc/platforms/powernv/opal-flash.c
@@ -437,10 +437,8 @@ static int alloc_image_buf(char *buffer, size_t count)
}
image_data.data = vzalloc(PAGE_ALIGN(image_data.size));
- if (!image_data.data) {
- pr_err("%s : Failed to allocate memory\n", __func__);
+ if (!image_data.data)
return -ENOMEM;
- }
/* Pin memory */
addr = image_data.data;
diff --git a/arch/powerpc/platforms/powernv/opal-hmi.c
b/arch/powerpc/platforms/powernv/opal-hmi.c
index d78fed728cdf..b7dfe41a0a96 100644
--- a/arch/powerpc/platforms/powernv/opal-hmi.c
+++ b/arch/powerpc/platforms/powernv/opal-hmi.c
@@ -310,10 +310,9 @@ static int opal_handle_hmi_event(struct notifier_block *nb,
/* Delay the logging of HMI events to workqueue. */
msg_node = kzalloc(sizeof(*msg_node), GFP_ATOMIC);
- if (!msg_node) {
- pr_err("HMI: out of memory, Opal message event not handled\n");
+ if (!msg_node)
return -ENOMEM;
- }
+
memcpy(&msg_node->hmi_evt, hmi_evt, sizeof(struct OpalHMIEvent));
spin_lock_irqsave(&opal_hmi_evt_lock, flags);
diff --git a/arch/powerpc/platforms/powernv/opal-memory-errors.c
b/arch/powerpc/platforms/powernv/opal-memory-errors.c
index 4495f428b500..8d76c05252d1 100644
--- a/arch/powerpc/platforms/powernv/opal-memory-errors.c
+++ b/arch/powerpc/platforms/powernv/opal-memory-errors.c
@@ -107,11 +107,9 @@ static int opal_memory_err_event(struct notifier_block *nb,
return 0;
msg_node = kzalloc(sizeof(*msg_node), GFP_ATOMIC);
- if (!msg_node) {
- pr_err("MEMORY_ERROR: out of memory, Opal message event not"
- "handled\n");
+ if (!msg_node)
return -ENOMEM;
- }
+
memcpy(&msg_node->msg, msg, sizeof(struct opal_msg));
spin_lock_irqsave(&opal_mem_err_lock, flags);
diff --git a/arch/powerpc/platforms/powernv/opal-sysparam.c
b/arch/powerpc/platforms/powernv/opal-sysparam.c
index 23fb6647dced..1df05efe55a1 100644
--- a/arch/powerpc/platforms/powernv/opal-sysparam.c
+++ b/arch/powerpc/platforms/powernv/opal-sysparam.c
@@ -184,11 +184,8 @@ void __init opal_sys_param_init(void)
/* Allocate big enough buffer for any get/set transactions */
param_data_buf = kzalloc(MAX_PARAM_DATA_LEN, GFP_KERNEL);
- if (!param_data_buf) {
- pr_err("SYSPARAM: Failed to allocate memory for param data "
- "buf\n");
+ if (!param_data_buf)
goto out_kobj_put;
- }
/* Number of parameters exposed through DT */
count = of_property_count_strings(sysparam, "param-name");
@@ -199,25 +196,16 @@ void __init opal_sys_param_init(void)
}
id = kzalloc(sizeof(*id) * count, GFP_KERNEL);
- if (!id) {
-
[PATCH 2/3] powerpc-opal: Improve 12 size determinations
From: Markus Elfring
Date: Tue, 17 Oct 2017 13:20:19 +0200
Replace the specification of data types by variable references
as the parameter for the operator "sizeof" to make the corresponding size
determination a bit safer according to the Linux coding style convention.
Signed-off-by: Markus Elfring
---
arch/powerpc/platforms/powernv/opal-flash.c | 4 ++--
arch/powerpc/platforms/powernv/opal-hmi.c | 2 +-
arch/powerpc/platforms/powernv/opal-imc.c | 10 +-
arch/powerpc/platforms/powernv/opal-memory-errors.c | 2 +-
arch/powerpc/platforms/powernv/opal-psr.c | 2 +-
arch/powerpc/platforms/powernv/opal-sensor-groups.c | 4 ++--
arch/powerpc/platforms/powernv/opal-xscom.c | 2 +-
7 files changed, 13 insertions(+), 13 deletions(-)
diff --git a/arch/powerpc/platforms/powernv/opal-flash.c
b/arch/powerpc/platforms/powernv/opal-flash.c
index 77d564266a59..9c6b54c86d71 100644
--- a/arch/powerpc/platforms/powernv/opal-flash.c
+++ b/arch/powerpc/platforms/powernv/opal-flash.c
@@ -418,12 +418,12 @@ static int alloc_image_buf(char *buffer, size_t count)
void *addr;
int size;
- if (count < sizeof(struct image_header_t)) {
+ if (count < sizeof(image_header)) {
pr_warn("FLASH: Invalid candidate image\n");
return -EINVAL;
}
- memcpy(&image_header, (void *)buffer, sizeof(struct image_header_t));
+ memcpy(&image_header, (void *)buffer, sizeof(image_header));
image_data.size = be32_to_cpu(image_header.size);
pr_debug("FLASH: Candidate image size = %u\n", image_data.size);
diff --git a/arch/powerpc/platforms/powernv/opal-hmi.c
b/arch/powerpc/platforms/powernv/opal-hmi.c
index b7dfe41a0a96..92b1e12a7eed 100644
--- a/arch/powerpc/platforms/powernv/opal-hmi.c
+++ b/arch/powerpc/platforms/powernv/opal-hmi.c
@@ -313,7 +313,7 @@ static int opal_handle_hmi_event(struct notifier_block *nb,
if (!msg_node)
return -ENOMEM;
- memcpy(&msg_node->hmi_evt, hmi_evt, sizeof(struct OpalHMIEvent));
+ memcpy(&msg_node->hmi_evt, hmi_evt, sizeof(*hmi_evt));
spin_lock_irqsave(&opal_hmi_evt_lock, flags);
list_add(&msg_node->list, &opal_hmi_evt_list);
diff --git a/arch/powerpc/platforms/powernv/opal-imc.c
b/arch/powerpc/platforms/powernv/opal-imc.c
index 21f6531fae20..2302cfb953bd 100644
--- a/arch/powerpc/platforms/powernv/opal-imc.c
+++ b/arch/powerpc/platforms/powernv/opal-imc.c
@@ -38,11 +38,11 @@ static int imc_get_mem_addr_nest(struct device_node *node,
if (nr_chips <= 0)
return -ENODEV;
- base_addr_arr = kcalloc(nr_chips, sizeof(u64), GFP_KERNEL);
+ base_addr_arr = kcalloc(nr_chips, sizeof(*base_addr_arr), GFP_KERNEL);
if (!base_addr_arr)
return -ENOMEM;
- chipid_arr = kcalloc(nr_chips, sizeof(u32), GFP_KERNEL);
+ chipid_arr = kcalloc(nr_chips, sizeof(*chipid_arr), GFP_KERNEL);
if (!chipid_arr)
return -ENOMEM;
@@ -53,8 +53,8 @@ static int imc_get_mem_addr_nest(struct device_node *node,
nr_chips))
goto error;
- pmu_ptr->mem_info = kcalloc(nr_chips, sizeof(struct imc_mem_info),
- GFP_KERNEL);
+ pmu_ptr->mem_info = kcalloc(nr_chips, sizeof(*pmu_ptr->mem_info),
+ GFP_KERNEL);
if (!pmu_ptr->mem_info)
goto error;
@@ -88,7 +88,7 @@ static int imc_pmu_create(struct device_node *parent, int
pmu_index, int domain)
u32 offset;
/* memory for pmu */
- pmu_ptr = kzalloc(sizeof(struct imc_pmu), GFP_KERNEL);
+ pmu_ptr = kzalloc(sizeof(*pmu_ptr), GFP_KERNEL);
if (!pmu_ptr)
return -ENOMEM;
diff --git a/arch/powerpc/platforms/powernv/opal-memory-errors.c
b/arch/powerpc/platforms/powernv/opal-memory-errors.c
index 8d76c05252d1..875e80ab96d9 100644
--- a/arch/powerpc/platforms/powernv/opal-memory-errors.c
+++ b/arch/powerpc/platforms/powernv/opal-memory-errors.c
@@ -110,7 +110,7 @@ static int opal_memory_err_event(struct notifier_block *nb,
if (!msg_node)
return -ENOMEM;
- memcpy(&msg_node->msg, msg, sizeof(struct opal_msg));
+ memcpy(&msg_node->msg, msg, sizeof(msg_node->msg));
spin_lock_irqsave(&opal_mem_err_lock, flags);
list_add(&msg_node->list, &opal_memory_err_list);
diff --git a/arch/powerpc/platforms/powernv/opal-psr.c
b/arch/powerpc/platforms/powernv/opal-psr.c
index 7313b7fc9071..74986b35cf77 100644
--- a/arch/powerpc/platforms/powernv/opal-psr.c
+++ b/arch/powerpc/platforms/powernv/opal-psr.c
@@ -136,7 +136,7 @@ void __init opal_psr_init(void)
return;
}
- psr_attrs = kcalloc(of_get_child_count(psr), sizeof(struct psr_attr),
+ psr_attrs = kcalloc(of_get_child_count(psr), siz
[PATCH 3/3] powerpc-opal: Fix a typo in a comment line of two file headers
From: Markus Elfring Date: Tue, 17 Oct 2017 13:31:42 +0200 Fix a word in these descriptions. Signed-off-by: Markus Elfring --- arch/powerpc/platforms/powernv/opal-hmi.c | 2 +- arch/powerpc/platforms/powernv/opal-memory-errors.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/arch/powerpc/platforms/powernv/opal-hmi.c b/arch/powerpc/platforms/powernv/opal-hmi.c index 92b1e12a7eed..481a004b1b6d 100644 --- a/arch/powerpc/platforms/powernv/opal-hmi.c +++ b/arch/powerpc/platforms/powernv/opal-hmi.c @@ -1,5 +1,5 @@ /* - * OPAL hypervisor Maintenance interrupt handling support in PowreNV. + * OPAL hypervisor Maintenance interrupt handling support in PowerNV. * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by diff --git a/arch/powerpc/platforms/powernv/opal-memory-errors.c b/arch/powerpc/platforms/powernv/opal-memory-errors.c index 875e80ab96d9..fd500b379560 100644 --- a/arch/powerpc/platforms/powernv/opal-memory-errors.c +++ b/arch/powerpc/platforms/powernv/opal-memory-errors.c @@ -1,5 +1,5 @@ /* - * OPAL asynchronus Memory error handling support in PowreNV. + * OPAL asynchronus Memory error handling support in PowerNV. * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by -- 2.14.2
[PATCH 0/3] PowerNV-PCI: Adjustments for two function implementations
From: Markus Elfring Date: Tue, 17 Oct 2017 17:27:37 +0200 Three update suggestions were taken into account from static source code analysis. Markus Elfring (3): Delete an error message for a failed memory allocation in pnv_ioda_pick_m64_pe() Use common code in pnv_ioda_pick_m64_pe() Improve a size determination in pnv_pci_init_ioda_phb() arch/powerpc/platforms/powernv/pci-ioda.c | 17 +++-- 1 file changed, 7 insertions(+), 10 deletions(-) -- 2.14.2
[PATCH 1/3] powernv/pci: Delete an error message for a failed memory allocation in pnv_ioda_pick_m64_pe()
From: Markus Elfring
Date: Tue, 17 Oct 2017 16:52:43 +0200
Omit an extra message for a memory allocation failure in this function.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring
---
arch/powerpc/platforms/powernv/pci-ioda.c | 5 +
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c
b/arch/powerpc/platforms/powernv/pci-ioda.c
index fb5cd7511189..17c0330bb059 100644
--- a/arch/powerpc/platforms/powernv/pci-ioda.c
+++ b/arch/powerpc/platforms/powernv/pci-ioda.c
@@ -358,11 +358,8 @@ static struct pnv_ioda_pe *pnv_ioda_pick_m64_pe(struct
pci_bus *bus, bool all)
/* Allocate bitmap */
size = _ALIGN_UP(phb->ioda.total_pe_num / 8, sizeof(unsigned long));
pe_alloc = kzalloc(size, GFP_KERNEL);
- if (!pe_alloc) {
- pr_warn("%s: Out of memory !\n",
- __func__);
+ if (!pe_alloc)
return NULL;
- }
/* Figure out reserved PE numbers by the PE */
pnv_ioda_reserve_m64_pe(bus, pe_alloc, all);
--
2.14.2
[PATCH 2/3] powernv/pci: Use common code in pnv_ioda_pick_m64_pe()
From: Markus Elfring
Date: Tue, 17 Oct 2017 17:07:54 +0200
Add a jump target so that a bit of code can be better reused
at the end of this function.
Signed-off-by: Markus Elfring
---
arch/powerpc/platforms/powernv/pci-ioda.c | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c
b/arch/powerpc/platforms/powernv/pci-ioda.c
index 17c0330bb059..98d9435240f4 100644
--- a/arch/powerpc/platforms/powernv/pci-ioda.c
+++ b/arch/powerpc/platforms/powernv/pci-ioda.c
@@ -364,21 +364,20 @@ static struct pnv_ioda_pe *pnv_ioda_pick_m64_pe(struct
pci_bus *bus, bool all)
/* Figure out reserved PE numbers by the PE */
pnv_ioda_reserve_m64_pe(bus, pe_alloc, all);
+ master_pe = NULL;
+
/*
* the current bus might not own M64 window and that's all
* contributed by its child buses. For the case, we needn't
* pick M64 dependent PE#.
*/
- if (bitmap_empty(pe_alloc, phb->ioda.total_pe_num)) {
- kfree(pe_alloc);
- return NULL;
- }
+ if (bitmap_empty(pe_alloc, phb->ioda.total_pe_num))
+ goto free_pe;
/*
* Figure out the master PE and put all slave PEs to master
* PE's list to form compound PE.
*/
- master_pe = NULL;
i = -1;
while ((i = find_next_bit(pe_alloc, phb->ioda.total_pe_num, i + 1)) <
phb->ioda.total_pe_num) {
@@ -416,6 +415,7 @@ static struct pnv_ioda_pe *pnv_ioda_pick_m64_pe(struct
pci_bus *bus, bool all)
}
}
+free_pe:
kfree(pe_alloc);
return master_pe;
}
--
2.14.2
[PATCH 3/3] powernv/pci: Improve a size determination in pnv_pci_init_ioda_phb()
From: Markus Elfring
Date: Tue, 17 Oct 2017 17:18:10 +0200
Replace the specification of a data structure by a pointer dereference
as the parameter for the operator "sizeof" to make the corresponding size
determination a bit safer according to the Linux coding style convention.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring
---
arch/powerpc/platforms/powernv/pci-ioda.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c
b/arch/powerpc/platforms/powernv/pci-ioda.c
index 98d9435240f4..2febdf06a237 100644
--- a/arch/powerpc/platforms/powernv/pci-ioda.c
+++ b/arch/powerpc/platforms/powernv/pci-ioda.c
@@ -3802,7 +3802,7 @@ static void __init pnv_pci_init_ioda_phb(struct
device_node *np,
phb_id = be64_to_cpup(prop64);
pr_debug(" PHB-ID : 0x%016llx\n", phb_id);
- phb = memblock_virt_alloc(sizeof(struct pnv_phb), 0);
+ phb = memblock_virt_alloc(sizeof(*phb), 0);
/* Allocate PCI controller */
phb->hose = hose = pcibios_alloc_controller(np);
--
2.14.2
Re: [PATCH 0/4] char-TPM: Adjustments for ten function implementations
>>> Fixes is only for bug fixes. These don't fix any bugs. >> >> How do you distinguish these in questionable source code >> from other error categories or software weaknesses? > > A style change is one that doesn't change the effect of the execution. This can occasionally be fine, can't it? > These don't actually even change the assembly, How did you check it? I would expect that there are useful run time effects to consider for three proposed update steps (in this patch series). > so there's programmatic proof they're not fixing anything. I find that the software refactoring “Improve a size determination in nine functions” should fit to this observation (while the source code can become a bit better). > Bug means potentially user visible fault. Thanks for your constructive feedback. Regards, Markus
Re: char/tpm: Improve a size determination in nine functions
>> p = kmalloc(sizeof(*p), ...); >> >> The alternative form where struct name is spelled out hurts readability and >> introduces an opportunity for a bug when the pointer variable type is changed >> but the corresponding sizeof that is passed to a memory allocator is not. > > True, thanks for the reminder. Will it trigger further software development considerations (besides my contributions)? > Is this common in new code? Do you start an official survey here? > Is there a script/ or some other automated way of catching this usage Yes. - I am using an approach for the semantic patch language. ;-) > before patches are upstreamed? I imagine that a corresponding source code analysis variant could be applied in more cases if sufficient acceptance could be achieved. > Just as you're doing here, the patch description should reference this > in the patch description. Do you find my wording “This issue was detected by using the Coccinelle software.” insufficient? Regards, Markus
[PATCH 0/2] PowerPC-PS3: Adjustments for three function implementations
From: Markus Elfring Date: Tue, 17 Oct 2017 20:22:44 +0200 Two update suggestions were taken into account from static source code analysis. Markus Elfring (2): Delete an error message for a failed memory allocation in update_flash_db() Improve a size determination in two functions arch/powerpc/platforms/ps3/mm.c | 6 ++ arch/powerpc/platforms/ps3/os-area.c | 4 +--- 2 files changed, 3 insertions(+), 7 deletions(-) -- 2.14.2
[PATCH 1/2] powerpc-ps3: Delete an error message for a failed memory allocation in update_flash_db()
From: Markus Elfring
Date: Tue, 17 Oct 2017 20:00:31 +0200
Omit an extra message for a memory allocation failure in this function.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring
---
arch/powerpc/platforms/ps3/os-area.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/arch/powerpc/platforms/ps3/os-area.c
b/arch/powerpc/platforms/ps3/os-area.c
index 3db53e8aff92..f2a8875e25d1 100644
--- a/arch/powerpc/platforms/ps3/os-area.c
+++ b/arch/powerpc/platforms/ps3/os-area.c
@@ -625,10 +625,8 @@ static int update_flash_db(void)
/* Read in header and db from flash. */
header = kmalloc(buf_len, GFP_KERNEL);
- if (!header) {
- pr_debug("%s: kmalloc failed\n", __func__);
+ if (!header)
return -ENOMEM;
- }
count = os_area_flash_read(header, buf_len, 0);
if (count < 0) {
--
2.14.2
[PATCH 2/2] powerpc-ps3: Improve a size determination in two functions
From: Markus Elfring
Date: Tue, 17 Oct 2017 20:15:17 +0200
Replace the specification of data structures by pointer dereferences
as the parameter for the operator "sizeof" to make the corresponding size
determination a bit safer according to the Linux coding style convention.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring
---
arch/powerpc/platforms/ps3/mm.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/platforms/ps3/mm.c b/arch/powerpc/platforms/ps3/mm.c
index b0f34663b1ae..257de451e324 100644
--- a/arch/powerpc/platforms/ps3/mm.c
+++ b/arch/powerpc/platforms/ps3/mm.c
@@ -524,8 +524,7 @@ static int dma_sb_map_pages(struct ps3_dma_region *r,
unsigned long phys_addr,
int result;
struct dma_chunk *c;
- c = kzalloc(sizeof(struct dma_chunk), GFP_ATOMIC);
-
+ c = kzalloc(sizeof(*c), GFP_ATOMIC);
if (!c) {
result = -ENOMEM;
goto fail_alloc;
@@ -570,8 +569,7 @@ static int dma_ioc0_map_pages(struct ps3_dma_region *r,
unsigned long phys_addr,
DBG(KERN_ERR "%s: phy=%#lx, lpar%#lx, len=%#lx\n", __func__,
phys_addr, ps3_mm_phys_to_lpar(phys_addr), len);
- c = kzalloc(sizeof(struct dma_chunk), GFP_ATOMIC);
-
+ c = kzalloc(sizeof(*c), GFP_ATOMIC);
if (!c) {
result = -ENOMEM;
goto fail_alloc;
--
2.14.2
Re: char/tpm: Improve a size determination in nine functions
>> Do you find my wording “This issue was detected by using the >> Coccinelle software.” insufficient? > > The question is not whether it is insufficient, but whether it is appropriate. I am curious on how our corresponding discussion will evolve further. > Detecting Coccinelle issues is one step. The next step is deciding > what to do with them. Will the clarification achieve any more useful results? > Up to now, these messages have been sent out as informational, not as patches. I sent some update suggestions as patches also in this series (as usual). > Before sending patches to change existing code, address the "problem" > so that it doesn't continue to happen. It might be very challenging to achieve such a goal. > Only afterwards is it appropriate to discuss what to do with existing code. I would prefer to get corresponding improvements in both areas in parallel (if it is generally possible). Regards, Markus
Re: char/tpm: Improve a size determination in nine functions
>> I imagine that a corresponding source code analysis variant could be applied >> in more cases if sufficient acceptance could be achieved. > > So, then instead of still keeping people busy with this noise you better > start doing something like CI integration with that for *new* code? There are various software development challenges to consider. > I'm pretty sure you may also exercise your achievements on > drivers/staging where it would be honored. I am waiting for several improvements also for software components in this area for a while. Would you like to take another look at these change possibilities? > Have you talked to Fengguang (0-day LKP)? Not directly for this topic so far. > Have you talked to Arnd (I think he is related to kernel-ci)? I am curious on how he will respond to remaining open issues. Regards, Markus
Re: char-TPM: Adjustments for ten function implementations
> The printk removals do change the objects. > > The value of that type of change is only for resource limited systems. I imagine that such small code adjustments are also useful for other systems. > Markus' changelogs leave much to be desired. Would you like to help more to improve the provided information for the shown change patterns? Regards, Markus
Re: char-TPM: Adjustments for ten function implementations
>> I imagine that such small code adjustments are also useful for other systems. > > Your imagination and mine differ. This can generally be. > Where do you _think_ it matters? It seems that this discussion branch referred still to my cover letter for possible changes in the TPM software area. The four update steps (in this patch series) demonstrate different change possibilities which could be desired. Would you like to distinguish them a bit more? > For instance, nothing about > > sizeof(type) > vs > sizeof(*ptr) > > makes it easier for a human to read the code. I could agree to this view (in the general short form). But nine statements became shorter in the concrete update suggestion so that such a reduction could help the trained eyes of some software developers and code reviewers. > This class of change now require a syntactic parser > to find instances of the use of type where previously > a grep or equivalent tool worked well. Does the Linux coding style convention prefer safety over this data processing concern? >>> Markus' changelogs leave much to be desired. >> >> Would you like to help more to improve the provided information >> for the shown change patterns? > > I've done that for you far too many times already. I got an other impression. You gave constructive feedback (also for me) occasionally. There were a few cases where a desired agreement was not achieved so far. > Your changelogs need to detail _why_ something is being done, I could improve descriptions if involved information sources could also become clearer and really safe. > not describe any tool used to perform or find a > particular instance of any change. This part refers to a bit of attribution. Regards, Markus
Re: Adjusting further size determinations?
> Ugly grep follows: > > $ grep -rohP --include=*.[ch] "\w+\s*=\s*[kv].alloc\s*\(\s*sizeof.*," * | \ > sed -r -e 's/(\w+)\s*=\s*[kv].alloc\s*\(\s*sizeof\s*\(\s*\*\s*\1\s*\)/foo = > k.alloc(sizeof(*foo))/' \ > -e > 's/(\w+)\s*=\s*[kv].alloc\s*\(\s*sizeof\s*\(\s*struct\s+\w+\s*\)/foo = > k.alloc(sizeof(struct foo))/' | \ > sort | uniq -c | sort -rn | head -2 >6123 foo = k.alloc(sizeof(*foo)), >3060 foo = k.alloc(sizeof(struct foo)), Would you like to get this ratio changed in any ways? Available development tools could help to improve the software situation in a desired direction, couldn't they? >> Unpleasant consequences are possible in both cases. How much do you care to reduce the failure probability further? Regards, Markus
Re: Adjusting further size determinations?
Unpleasant consequences are possible in both cases. >> How much do you care to reduce the failure probability further? > > Zero. I am interested to improve the software situation a bit more here. Regards, Markus
Re: Adjusting further size determinations?
>> If you want 'security' for kmalloc() then: >> >> #define KMALLOC_TYPE(flags) (type *)kmalloc(sizeof (type), flags) >> #define KMALLOC(ptr, flags) *(ptr) = KMALLOC_TYPE(typeof *(ptr), flags) Such an approach might help. >> and change: >> ptr = kmalloc(sizeof *ptr, flags); >> to: >> KMALLOC(&ptr, flags); >> >> But it is all churn for churn's sake. > > Please don't. Interesting … > Coccinelle won't find real problems with kmalloc any more if this is done. The corresponding source code analysis will become different (or more challenging) then. Are you still looking for related solutions? Regards, Markus
Re: char/tpm: Improve a size determination in nine functions
>> Do you find my wording “This issue was detected by using the >> Coccinelle software.” insufficient? > > This is fine for cover letter, not for the commits. I guess that there are more opinions available by other contributors for this aspect. > After your analysis software finds an issue you should manually analyze > what is wrong This view is generally fine. > and document that to the commit message. I tried it in a single paragraph so far (besides the reference for the tool). > This applies to sparse, coccinelle or any other tool. I find that further possibilities can be considered. > Tool-based commit messages are bad for commit history I disagree to this view. > where as clean description gives idea what was done > (if you have to maintain a GIT tree). How do you think about to offer any wording for an alternative which you would find better? > In my opinion tool is doing all the work but the part > that you should do is absent. Really? Regards, Markus
Re: char-TPM: Adjustments for ten function implementations
>>> A minor complaint: all commits are missing "Fixes:" tag. >> >> * Do you require it to be added to the commit messages? > > I don't require it. It's part of the development process: > > https://www.kernel.org/doc/html/v4.12/process/submitting-patches.html Yes. - But other contributors pointed the detail out again that not every change is qualified for using this tag. >> * Would you like to get a finer patch granularity then? > > I don't understand what you are asking. If you would insist on the addition of this tag to all my commits for the discussed patch series, I imagine that I would need to split the update step “Improve a size determination in nine functions” into smaller parts. >> * Do you find any more information missing? > > I think I already answered to this in my earlier responses > (commit messages). Partly. > I probably won't take "sizeof(*foo)" type of change even if it > is a recommended style if that is the only useful thing that the > commit does. How much do you care for the section “14) Allocating memory” in the document “coding-style.rst” then? Regards, Markus
Re: char/tpm: Improve a size determination in nine functions
> Commit message should just describe in plain text what you are doing Did other contributors find the wording “Replace …” > and why. and “… a bit safer according to the Linux coding style convention.” sufficient often enough already? Which description would you find more appropriate for this change pattern? Regards, Markus
Re: char/tpm: Improve a size determination in nine functions
> For 1/4 and 2/4: explain why the message can be omitted. Why did you not reply directly with this request for the update steps with the subject “Delete an error message for a failed memory allocation in tpm_…()”? https://patchwork.kernel.org/patch/10009405/ https://patchwork.kernel.org/patch/10009415/ I find that there can be difficulty to show an appropriate information source for the reasonable explanation of this change pattern. > Remove sentence about Coccinelle. I got the impression that there is a bit of value in such a kind of attribution. > That's all. I assume that there might be also some communication challenges involved. > 3/4: definitive NAK, too much noise compared to value. I tried to reduce deviations from the Linux coding style again. You do not like such an attempt for this software area so far. > 4/4: this a good commit message. Why did you not reply directly with this feedback for the update step “[PATCH 4/4] char/tpm: Less checks in tpm_ibmvtpm_probe() after error detection”? https://patchwork.kernel.org/patch/10009429/ https://lkml.kernel.org/r/<09a2c3a1-1b10-507d-a866-258b570f6...@users.sourceforge.net> > Requires a Tested-by before can be accepted, which I'm not able to give. I am curious on how this detail will evolve. Regards, Markus
Re: char/tpm: Improve a size determination in nine functions
> One more word of advice: send the three as separate patches. I do not see a need for an immediate resend at the moment. > My guess is that it takes a factor longer time to apply 4/4 > than other patches because there's more limited crowd who can test it. This is fine for me if somebody would like to integrate this update suggestion at all. How do you think about to separate replies better between affected update steps? Regards, Markus
Re: char/tpm: Delete an error message for a failed memory allocation in tpm_…()
>> Why did you not reply directly with this request for the update steps >> with the subject “Delete an error message for a failed memory allocation >> in tpm_…()”? >> >> https://patchwork.kernel.org/patch/10009405/ >> https://patchwork.kernel.org/patch/10009415/ >> >> I find that there can be difficulty to show an appropriate information >> source for the reasonable explanation of this change pattern. >> > > Shouldn't this information source for the explanation be the submitter? I offered a bit of information. I agree that it could become better eventually. > I'd hope they understand what it is they are submitting. I do this to some degree. ;-) But I would appreciate if I could refer to a single Linux document for this change pattern around questionable error messages. Would a corresponding link for an accepted explanation in the documentation be nice in this case? Regards, Markus
[PATCH 0/5] PowerPC-pSeries: Adjustments for seven function implementations
From: Markus Elfring Date: Wed, 18 Oct 2017 21:11:23 +0200 A few update suggestions were taken into account from static source code analysis. Markus Elfring (5): Delete five error messages for a failed memory allocation Improve nine size determinations Delete an unnecessary variable initialisation in iommu_pseries_alloc_group() Return directly after a failed kzalloc_node() in iommu_pseries_alloc_group() Less function calls in iommu_pseries_alloc_group() after error detection arch/powerpc/platforms/pseries/dlpar.c | 5 ++-- arch/powerpc/platforms/pseries/dtl.c | 5 +--- arch/powerpc/platforms/pseries/hvcserver.c | 7 ++ arch/powerpc/platforms/pseries/iommu.c | 40 -- arch/powerpc/platforms/pseries/vio.c | 9 +++ 5 files changed, 24 insertions(+), 42 deletions(-) -- 2.14.2
[PATCH 1/5] powerpc-pseries: Delete five error messages for a failed memory allocation
From: Markus Elfring
Date: Wed, 18 Oct 2017 16:39:01 +0200
Omit extra messages for a memory allocation failure in these functions.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring
---
arch/powerpc/platforms/pseries/dtl.c | 5 +
arch/powerpc/platforms/pseries/hvcserver.c | 2 --
arch/powerpc/platforms/pseries/iommu.c | 11 +++
arch/powerpc/platforms/pseries/vio.c | 4 +---
4 files changed, 5 insertions(+), 17 deletions(-)
diff --git a/arch/powerpc/platforms/pseries/dtl.c
b/arch/powerpc/platforms/pseries/dtl.c
index 18014cdeb590..467b9f578a7d 100644
--- a/arch/powerpc/platforms/pseries/dtl.c
+++ b/arch/powerpc/platforms/pseries/dtl.c
@@ -203,11 +203,8 @@ static int dtl_enable(struct dtl *dtl)
n_entries = dtl_buf_entries;
buf = kmem_cache_alloc_node(dtl_cache, GFP_KERNEL,
cpu_to_node(dtl->cpu));
- if (!buf) {
- printk(KERN_WARNING "%s: buffer alloc failed for cpu %d\n",
- __func__, dtl->cpu);
+ if (!buf)
return -ENOMEM;
- }
spin_lock(&dtl->lock);
rc = -EBUSY;
diff --git a/arch/powerpc/platforms/pseries/hvcserver.c
b/arch/powerpc/platforms/pseries/hvcserver.c
index 94a6e5612b0d..db2c20e65a58 100644
--- a/arch/powerpc/platforms/pseries/hvcserver.c
+++ b/arch/powerpc/platforms/pseries/hvcserver.c
@@ -177,8 +177,6 @@ int hvcs_get_partner_info(uint32_t unit_address, struct
list_head *head,
GFP_ATOMIC);
if (!next_partner_info) {
- printk(KERN_WARNING "HVCONSOLE: kmalloc() failed to"
- " allocate partner info struct.\n");
hvcs_free_partner_info(head);
return -ENOMEM;
}
diff --git a/arch/powerpc/platforms/pseries/iommu.c
b/arch/powerpc/platforms/pseries/iommu.c
index 7c181467d0ad..c92822fdf269 100644
--- a/arch/powerpc/platforms/pseries/iommu.c
+++ b/arch/powerpc/platforms/pseries/iommu.c
@@ -1063,19 +1063,14 @@ static u64 enable_ddw(struct pci_dev *dev, struct
device_node *pdn)
}
len = order_base_2(max_addr);
win64 = kzalloc(sizeof(struct property), GFP_KERNEL);
- if (!win64) {
- dev_info(&dev->dev,
- "couldn't allocate property for 64bit dma window\n");
+ if (!win64)
goto out_failed;
- }
+
win64->name = kstrdup(DIRECT64_PROPNAME, GFP_KERNEL);
win64->value = ddwprop = kmalloc(sizeof(*ddwprop), GFP_KERNEL);
win64->length = sizeof(*ddwprop);
- if (!win64->name || !win64->value) {
- dev_info(&dev->dev,
- "couldn't allocate property name and value\n");
+ if (!win64->name || !win64->value)
goto out_free_prop;
- }
ret = create_ddw(dev, ddw_avail, &create, page_shift, len);
if (ret != 0)
diff --git a/arch/powerpc/platforms/pseries/vio.c
b/arch/powerpc/platforms/pseries/vio.c
index 12277bc9fd9e..74b919040e68 100644
--- a/arch/powerpc/platforms/pseries/vio.c
+++ b/arch/powerpc/platforms/pseries/vio.c
@@ -1386,10 +1386,8 @@ struct vio_dev *vio_register_device_node(struct
device_node *of_node)
/* allocate a vio_dev for this node */
viodev = kzalloc(sizeof(struct vio_dev), GFP_KERNEL);
- if (viodev == NULL) {
- pr_warn("%s: allocation failure for VIO device.\n", __func__);
+ if (!viodev)
return NULL;
- }
/* we need the 'device_type' property, in order to match with drivers */
viodev->family = family;
--
2.14.2
[PATCH 2/5] powerpc-pseries: Improve nine size determinations
From: Markus Elfring
Date: Wed, 18 Oct 2017 18:18:11 +0200
Replace the specification of data structures by pointer dereferences
as the parameter for the operator "sizeof" to make the corresponding size
determination a bit safer according to the Linux coding style convention.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring
---
arch/powerpc/platforms/pseries/dlpar.c | 5 ++---
arch/powerpc/platforms/pseries/hvcserver.c | 5 ++---
arch/powerpc/platforms/pseries/iommu.c | 10 --
arch/powerpc/platforms/pseries/vio.c | 5 ++---
4 files changed, 10 insertions(+), 15 deletions(-)
diff --git a/arch/powerpc/platforms/pseries/dlpar.c
b/arch/powerpc/platforms/pseries/dlpar.c
index 6e35780c5962..dca88997cb4b 100644
--- a/arch/powerpc/platforms/pseries/dlpar.c
+++ b/arch/powerpc/platforms/pseries/dlpar.c
@@ -389,11 +389,10 @@ void queue_hotplug_event(struct pseries_hp_errorlog
*hp_errlog,
struct pseries_hp_work *work;
struct pseries_hp_errorlog *hp_errlog_copy;
- hp_errlog_copy = kmalloc(sizeof(struct pseries_hp_errorlog),
-GFP_KERNEL);
+ hp_errlog_copy = kmalloc(sizeof(*hp_errlog_copy), GFP_KERNEL);
memcpy(hp_errlog_copy, hp_errlog, sizeof(struct pseries_hp_errorlog));
- work = kmalloc(sizeof(struct pseries_hp_work), GFP_KERNEL);
+ work = kmalloc(sizeof(*work), GFP_KERNEL);
if (work) {
INIT_WORK((struct work_struct *)work, pseries_hp_work_fn);
work->errlog = hp_errlog_copy;
diff --git a/arch/powerpc/platforms/pseries/hvcserver.c
b/arch/powerpc/platforms/pseries/hvcserver.c
index db2c20e65a58..b85cca04dd7d 100644
--- a/arch/powerpc/platforms/pseries/hvcserver.c
+++ b/arch/powerpc/platforms/pseries/hvcserver.c
@@ -173,9 +173,8 @@ int hvcs_get_partner_info(uint32_t unit_address, struct
list_head *head,
/* This is a very small struct and will be freed soon in
* hvcs_free_partner_info(). */
- next_partner_info = kmalloc(sizeof(struct hvcs_partner_info),
- GFP_ATOMIC);
-
+ next_partner_info = kmalloc(sizeof(*next_partner_info),
+ GFP_ATOMIC);
if (!next_partner_info) {
hvcs_free_partner_info(head);
return -ENOMEM;
diff --git a/arch/powerpc/platforms/pseries/iommu.c
b/arch/powerpc/platforms/pseries/iommu.c
index c92822fdf269..b37d4fb20d1c 100644
--- a/arch/powerpc/platforms/pseries/iommu.c
+++ b/arch/powerpc/platforms/pseries/iommu.c
@@ -59,17 +59,15 @@ static struct iommu_table_group
*iommu_pseries_alloc_group(int node)
struct iommu_table *tbl = NULL;
struct iommu_table_group_link *tgl = NULL;
- table_group = kzalloc_node(sizeof(struct iommu_table_group), GFP_KERNEL,
- node);
+ table_group = kzalloc_node(sizeof(*table_group), GFP_KERNEL, node);
if (!table_group)
goto fail_exit;
- tbl = kzalloc_node(sizeof(struct iommu_table), GFP_KERNEL, node);
+ tbl = kzalloc_node(sizeof(*tbl), GFP_KERNEL, node);
if (!tbl)
goto fail_exit;
- tgl = kzalloc_node(sizeof(struct iommu_table_group_link), GFP_KERNEL,
- node);
+ tgl = kzalloc_node(sizeof(*tgl), GFP_KERNEL, node);
if (!tgl)
goto fail_exit;
@@ -1062,7 +1060,7 @@ static u64 enable_ddw(struct pci_dev *dev, struct
device_node *pdn)
goto out_failed;
}
len = order_base_2(max_addr);
- win64 = kzalloc(sizeof(struct property), GFP_KERNEL);
+ win64 = kzalloc(sizeof(*win64), GFP_KERNEL);
if (!win64)
goto out_failed;
diff --git a/arch/powerpc/platforms/pseries/vio.c
b/arch/powerpc/platforms/pseries/vio.c
index 74b919040e68..cf0939eb3aee 100644
--- a/arch/powerpc/platforms/pseries/vio.c
+++ b/arch/powerpc/platforms/pseries/vio.c
@@ -754,8 +754,7 @@ static int vio_cmo_bus_probe(struct vio_dev *viodev)
viodev->cmo.desired = VIO_CMO_MIN_ENT;
size = VIO_CMO_MIN_ENT;
- dev_ent = kmalloc(sizeof(struct vio_cmo_dev_entry),
- GFP_KERNEL);
+ dev_ent = kmalloc(sizeof(*dev_ent), GFP_KERNEL);
if (!dev_ent)
return -ENOMEM;
@@ -1385,7 +1384,7 @@ struct vio_dev *vio_register_device_node(struct
device_node *of_node)
}
/* allocate a vio_dev for this node */
- viodev = kzalloc(sizeof(struct vio_dev), GFP_KERNEL);
+ viodev = kzalloc(sizeof(*viodev), GFP_KERNEL);
if (!viodev)
return NULL;
--
2.14.2
[PATCH 3/5] powerpc-pseries: Delete an unnecessary variable initialisation in iommu_pseries_alloc_group()
From: Markus Elfring
Date: Wed, 18 Oct 2017 19:14:39 +0200
The variable "table_group" will be set to an appropriate pointer.
Thus omit the explicit initialisation at the beginning.
Signed-off-by: Markus Elfring
---
arch/powerpc/platforms/pseries/iommu.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/platforms/pseries/iommu.c
b/arch/powerpc/platforms/pseries/iommu.c
index b37d4fb20d1c..b6c12b8e3ace 100644
--- a/arch/powerpc/platforms/pseries/iommu.c
+++ b/arch/powerpc/platforms/pseries/iommu.c
@@ -55,7 +55,7 @@
static struct iommu_table_group *iommu_pseries_alloc_group(int node)
{
- struct iommu_table_group *table_group = NULL;
+ struct iommu_table_group *table_group;
struct iommu_table *tbl = NULL;
struct iommu_table_group_link *tgl = NULL;
--
2.14.2
[PATCH 4/5] powerpc-pseries: Return directly after a failed kzalloc_node() in iommu_pseries_alloc_group()
From: Markus Elfring Date: Wed, 18 Oct 2017 20:15:32 +0200 Return directly after a call of the function "kzalloc_node" failed at the beginning. Signed-off-by: Markus Elfring --- arch/powerpc/platforms/pseries/iommu.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/powerpc/platforms/pseries/iommu.c b/arch/powerpc/platforms/pseries/iommu.c index b6c12b8e3ace..207ff8351af1 100644 --- a/arch/powerpc/platforms/pseries/iommu.c +++ b/arch/powerpc/platforms/pseries/iommu.c @@ -61,7 +61,7 @@ static struct iommu_table_group *iommu_pseries_alloc_group(int node) table_group = kzalloc_node(sizeof(*table_group), GFP_KERNEL, node); if (!table_group) - goto fail_exit; + return NULL; tbl = kzalloc_node(sizeof(*tbl), GFP_KERNEL, node); if (!tbl) -- 2.14.2
[PATCH 5/5] powerpc-pseries: Less function calls in iommu_pseries_alloc_group() after error detection
From: Markus Elfring
Date: Wed, 18 Oct 2017 20:48:52 +0200
The kfree() function was called in up to two cases by the
iommu_pseries_alloc_group() function during error handling
even if the passed variable contained a null pointer.
* Adjust jump targets according to the Linux coding style convention.
* Delete initialisations for the variables "tbl" and "tgl"
which became unnecessary with this refactoring.
Signed-off-by: Markus Elfring
---
arch/powerpc/platforms/pseries/iommu.c | 15 +++
1 file changed, 7 insertions(+), 8 deletions(-)
diff --git a/arch/powerpc/platforms/pseries/iommu.c
b/arch/powerpc/platforms/pseries/iommu.c
index 207ff8351af1..13b424f34039 100644
--- a/arch/powerpc/platforms/pseries/iommu.c
+++ b/arch/powerpc/platforms/pseries/iommu.c
@@ -56,8 +56,8 @@
static struct iommu_table_group *iommu_pseries_alloc_group(int node)
{
struct iommu_table_group *table_group;
- struct iommu_table *tbl = NULL;
- struct iommu_table_group_link *tgl = NULL;
+ struct iommu_table *tbl;
+ struct iommu_table_group_link *tgl;
table_group = kzalloc_node(sizeof(*table_group), GFP_KERNEL, node);
if (!table_group)
@@ -65,11 +65,11 @@ static struct iommu_table_group
*iommu_pseries_alloc_group(int node)
tbl = kzalloc_node(sizeof(*tbl), GFP_KERNEL, node);
if (!tbl)
- goto fail_exit;
+ goto free_group;
tgl = kzalloc_node(sizeof(*tgl), GFP_KERNEL, node);
if (!tgl)
- goto fail_exit;
+ goto free_table;
INIT_LIST_HEAD_RCU(&tbl->it_group_list);
kref_init(&tbl->it_kref);
@@ -80,11 +80,10 @@ static struct iommu_table_group
*iommu_pseries_alloc_group(int node)
return table_group;
-fail_exit:
- kfree(tgl);
- kfree(table_group);
+free_table:
kfree(tbl);
-
+free_group:
+ kfree(table_group);
return NULL;
}
--
2.14.2
Re: [PATCH 3/5] powerpc-pseries: Delete an unnecessary variable initialisation in iommu_pseries_alloc_group()
>> static struct iommu_table_group *iommu_pseries_alloc_group(int node)
>> {
>> -struct iommu_table_group *table_group = NULL;
>> +struct iommu_table_group *table_group;
>> struct iommu_table *tbl = NULL;
>> struct iommu_table_group_link *tgl = NULL;
>>
>
> I think initializing pointers to NULL is generally a good idea.
This one would also not be needed if the call of the function “kzalloc_node”
could be specified in the same statement.
> Removing these initializers adds no value, to the contrary.
This small update step is just a “preparation” for the subsequent two
suggestions
in this patch series.
Regards,
Markus
Re: [PATCH 4/5] powerpc-pseries: Return directly after a failed kzalloc_node() in iommu_pseries_alloc_group()
>> @@ -61,7 +61,7 @@ static struct iommu_table_group >> *iommu_pseries_alloc_group(int node) >> table_group = kzalloc_node(sizeof(*table_group), GFP_KERNEL, >> node); if (!table_group) >> -goto fail_exit; >> +return NULL; >> >> tbl = kzalloc_node(sizeof(*tbl), GFP_KERNEL, node); >> if (!tbl) > > I have seen quite a few fixes that do inverse of this patch after a > piece of code allocating some extra piece of memory was added before > code that just returns on fail because it is the first allocation in > the function. > > This is not useful. How do you think about an information from the section “7) Centralized exiting of functions” in the document “coding-style.rst” then? “… If there is no cleanup needed then just return directly. …” > A final fail_exit that frees everything that could have been allocated > is much better. I got an other software development opinion for such use cases. I prefer only required function calls there. Regards, Markus
Re: [PATCH 4/4] char/tpm: Less checks in tpm_ibmvtpm_probe() after error detection
>> @@ -683,13 +683,10 @@ static int tpm_ibmvtpm_probe(struct vio_dev *vio_dev,
>> reg_crq_cleanup:
>> dma_unmap_single(dev, ibmvtpm->crq_dma_handle,
>> CRQ_RES_BUF_SIZE, DMA_BIDIRECTIONAL);
>> -cleanup:
>> -if (ibmvtpm) {
>> -if (crq_q->crq_addr)
>> -free_page((unsigned long)crq_q->crq_addr);
>> -kfree(ibmvtpm);
>> -}
>> -
>
> I think a single cleanup section is better than many labels that just
> avoid a single null check.
I proposed to delete two unnecessary condition checks together with
an adjustment of jump targets.
Regards,
Markus
Re: Circumstances for using the tag “Fixes” (or not)
>>> The "Fixes" tag is an indication that the patch should be backported. >> >> No it's not that strong. It's an indication that the patch fixes another >> commit, which may or may not mean it should be backported depending on >> the preferences of the backporter. If it *does* need backporting then >> the Fixes tag helps identify where it should go. > > Thank you for setting the record straight. > >> The doco is actually pretty well worded IMO: It seems that there are still interpretations left over for further clarification. Would any porters dare to distribute the deletion of questionable condition checks (and special error messages) to more software versions? >> >> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst#n183 >> >> If your patch fixes a bug in a specific commit, e.g. you found an issue >> using >> ``git bisect``, please use the 'Fixes:' tag with the first 12 characters of >> the SHA-1 ID, and the one line summary. Would you dare to categorise any software inefficiencies with a bug type? Regards, Markus
Re: char/tpm: Less checks in tpm_ibmvtpm_probe() after error detection
>> If the code doing the allocation is changed in the future the single >> cleanup can stay whereas multiple labels have to be rewritten again. > > No, they don't unless you choose bad label names. Perhaps numbered > labels? We don't get a lot of those in the kernel any more. Label > name should be based on what the label does. Often I see bad label > names like generic labels: > > foo = kmalloc(); > if (!foo) > goto out; > > What is out going to do? Another common anti-pattern is come-from > labels: > > foo = kmalloc(); > if (!foo) > goto kmalloc_failed; > > Obviously, we can see from the if statement that the alloc failed and > you *just* know the next line is going to be is going to be: > > if (invalid) > goto kmalloc_failed; > > Which is wrong because kmalloc didn't fail... But if the label name is > based on what it does then, when you add or a remove an allocation, you > just have to edit the one thing. Would you be interested in an update on a topic like “Source code review around jump label usage”? https://lkml.org/lkml/2015/12/11/378 Regards, Markus
[PATCH 0/3] tty/serial/ucc_uart: Adjustments for two functions
From: Markus Elfring Date: Wed, 6 Dec 2017 22:06:44 +0100 Three update suggestions were taken into account from static source code analysis. Markus Elfring (3): Delete an error message for a failed memory allocation in ucc_uart_probe() Improve a size determination in ucc_uart_probe() Fix a typo in a comment line drivers/tty/serial/ucc_uart.c | 8 +++- 1 file changed, 3 insertions(+), 5 deletions(-) -- 2.15.1
[PATCH 1/3] tty/serial/ucc_uart: Delete an error message for a failed memory allocation in ucc_uart_probe()
From: Markus Elfring
Date: Wed, 6 Dec 2017 21:41:14 +0100
Omit an extra message for a memory allocation failure in this function.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring
---
drivers/tty/serial/ucc_uart.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/tty/serial/ucc_uart.c b/drivers/tty/serial/ucc_uart.c
index 2b6376e6e5ad..48364f0eba72 100644
--- a/drivers/tty/serial/ucc_uart.c
+++ b/drivers/tty/serial/ucc_uart.c
@@ -1254,10 +1254,8 @@ static int ucc_uart_probe(struct platform_device *ofdev)
}
qe_port = kzalloc(sizeof(struct uart_qe_port), GFP_KERNEL);
- if (!qe_port) {
- dev_err(&ofdev->dev, "can't allocate QE port structure\n");
+ if (!qe_port)
return -ENOMEM;
- }
/* Search for IRQ and mapbase */
ret = of_address_to_resource(np, 0, &res);
--
2.15.1
[PATCH 2/3] tty/serial/ucc_uart: Improve a size determination in ucc_uart_probe()
From: Markus Elfring Date: Wed, 6 Dec 2017 21:45:32 +0100 Replace the specification of a data structure by a pointer dereference as the parameter for the operator "sizeof" to make the corresponding size determination a bit safer according to the Linux coding style convention. This issue was detected by using the Coccinelle software. Signed-off-by: Markus Elfring --- drivers/tty/serial/ucc_uart.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/tty/serial/ucc_uart.c b/drivers/tty/serial/ucc_uart.c index 48364f0eba72..a9ce652899aa 100644 --- a/drivers/tty/serial/ucc_uart.c +++ b/drivers/tty/serial/ucc_uart.c @@ -1253,7 +1253,7 @@ static int ucc_uart_probe(struct platform_device *ofdev) } } - qe_port = kzalloc(sizeof(struct uart_qe_port), GFP_KERNEL); + qe_port = kzalloc(sizeof(*qe_port), GFP_KERNEL); if (!qe_port) return -ENOMEM; -- 2.15.1
[PATCH 3/3] tty/serial/ucc_uart: Fix a typo in a comment line
From: Markus Elfring Date: Wed, 6 Dec 2017 21:56:28 +0100 Add a missing character in a function name of this description. Signed-off-by: Markus Elfring --- drivers/tty/serial/ucc_uart.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/tty/serial/ucc_uart.c b/drivers/tty/serial/ucc_uart.c index a9ce652899aa..d0f139d897a2 100644 --- a/drivers/tty/serial/ucc_uart.c +++ b/drivers/tty/serial/ucc_uart.c @@ -1154,7 +1154,7 @@ static unsigned int soc_info(unsigned int *rev_h, unsigned int *rev_l) } /* - * requst_firmware_nowait() callback function + * request_firmware_nowait() callback function * * This function is called by the kernel when a firmware is made available, * or if it times out waiting for the firmware. -- 2.15.1
[PATCH 0/3] SoC/FSL/QE: Adjustments for three function implementations
From: Markus Elfring Date: Wed, 13 Dec 2017 18:28:38 +0100 Three update suggestions were taken into account from static source code analysis. Markus Elfring (3): Delete an error message for a failed memory allocation in three functions Improve a size determination in two functions Use common error handling code in ucc_fast_init() drivers/soc/fsl/qe/gpio.c | 4 +--- drivers/soc/fsl/qe/ucc_fast.c | 39 ++- drivers/soc/fsl/qe/ucc_slow.c | 7 ++- 3 files changed, 21 insertions(+), 29 deletions(-) -- 2.15.1
[PATCH 1/3] soc/fsl/qe: Delete an error message for a failed memory allocation in three functions
From: Markus Elfring
Date: Wed, 13 Dec 2017 17:45:23 +0100
Omit an extra message for a memory allocation failure in these functions.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring
---
drivers/soc/fsl/qe/gpio.c | 4 +---
drivers/soc/fsl/qe/ucc_fast.c | 5 +
drivers/soc/fsl/qe/ucc_slow.c | 5 +
3 files changed, 3 insertions(+), 11 deletions(-)
diff --git a/drivers/soc/fsl/qe/gpio.c b/drivers/soc/fsl/qe/gpio.c
index 3b27075c21a7..9239cf176e67 100644
--- a/drivers/soc/fsl/qe/gpio.c
+++ b/drivers/soc/fsl/qe/gpio.c
@@ -143,10 +143,8 @@ struct qe_pin *qe_pin_request(struct device_node *np, int
index)
unsigned long flags;
qe_pin = kzalloc(sizeof(*qe_pin), GFP_KERNEL);
- if (!qe_pin) {
- pr_debug("%s: can't allocate memory\n", __func__);
+ if (!qe_pin)
return ERR_PTR(-ENOMEM);
- }
err = of_get_gpio(np, index);
if (err < 0)
diff --git a/drivers/soc/fsl/qe/ucc_fast.c b/drivers/soc/fsl/qe/ucc_fast.c
index 83d8d16e3a69..59b68bf4aebb 100644
--- a/drivers/soc/fsl/qe/ucc_fast.c
+++ b/drivers/soc/fsl/qe/ucc_fast.c
@@ -195,11 +195,8 @@ int ucc_fast_init(struct ucc_fast_info * uf_info, struct
ucc_fast_private ** ucc
}
uccf = kzalloc(sizeof(struct ucc_fast_private), GFP_KERNEL);
- if (!uccf) {
- printk(KERN_ERR "%s: Cannot allocate private data\n",
- __func__);
+ if (!uccf)
return -ENOMEM;
- }
/* Fill fast UCC structure */
uccf->uf_info = uf_info;
diff --git a/drivers/soc/fsl/qe/ucc_slow.c b/drivers/soc/fsl/qe/ucc_slow.c
index 9334bdbd9b30..fc91412e2300 100644
--- a/drivers/soc/fsl/qe/ucc_slow.c
+++ b/drivers/soc/fsl/qe/ucc_slow.c
@@ -153,11 +153,8 @@ int ucc_slow_init(struct ucc_slow_info * us_info, struct
ucc_slow_private ** ucc
}
uccs = kzalloc(sizeof(struct ucc_slow_private), GFP_KERNEL);
- if (!uccs) {
- printk(KERN_ERR "%s: Cannot allocate private data\n",
- __func__);
+ if (!uccs)
return -ENOMEM;
- }
/* Fill slow UCC structure */
uccs->us_info = us_info;
--
2.15.1
[PATCH 2/3] soc/fsl/qe: Improve a size determination in two functions
From: Markus Elfring Date: Wed, 13 Dec 2017 17:51:21 +0100 Replace the specification of data structures by pointer dereferences as the parameter for the operator "sizeof" to make the corresponding size determination a bit safer according to the Linux coding style convention. This issue was detected by using the Coccinelle software. Signed-off-by: Markus Elfring --- drivers/soc/fsl/qe/ucc_fast.c | 2 +- drivers/soc/fsl/qe/ucc_slow.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/soc/fsl/qe/ucc_fast.c b/drivers/soc/fsl/qe/ucc_fast.c index 59b68bf4aebb..2092bfdcf1bc 100644 --- a/drivers/soc/fsl/qe/ucc_fast.c +++ b/drivers/soc/fsl/qe/ucc_fast.c @@ -194,7 +194,7 @@ int ucc_fast_init(struct ucc_fast_info * uf_info, struct ucc_fast_private ** ucc return -EINVAL; } - uccf = kzalloc(sizeof(struct ucc_fast_private), GFP_KERNEL); + uccf = kzalloc(sizeof(*uccf), GFP_KERNEL); if (!uccf) return -ENOMEM; diff --git a/drivers/soc/fsl/qe/ucc_slow.c b/drivers/soc/fsl/qe/ucc_slow.c index fc91412e2300..c21a42c11080 100644 --- a/drivers/soc/fsl/qe/ucc_slow.c +++ b/drivers/soc/fsl/qe/ucc_slow.c @@ -152,7 +152,7 @@ int ucc_slow_init(struct ucc_slow_info * us_info, struct ucc_slow_private ** ucc return -EINVAL; } - uccs = kzalloc(sizeof(struct ucc_slow_private), GFP_KERNEL); + uccs = kzalloc(sizeof(*uccs), GFP_KERNEL); if (!uccs) return -ENOMEM; -- 2.15.1
[PATCH 3/3] soc/fsl/qe: Use common error handling code in ucc_fast_init()
From: Markus Elfring
Date: Wed, 13 Dec 2017 18:18:56 +0100
Add jump targets so that a bit of exception handling can be better reused
at the end of this function.
Signed-off-by: Markus Elfring
---
drivers/soc/fsl/qe/ucc_fast.c | 32
1 file changed, 16 insertions(+), 16 deletions(-)
diff --git a/drivers/soc/fsl/qe/ucc_fast.c b/drivers/soc/fsl/qe/ucc_fast.c
index 2092bfdcf1bc..268dfc5dc89b 100644
--- a/drivers/soc/fsl/qe/ucc_fast.c
+++ b/drivers/soc/fsl/qe/ucc_fast.c
@@ -269,8 +269,7 @@ int ucc_fast_init(struct ucc_fast_info * uf_info, struct
ucc_fast_private ** ucc
printk(KERN_ERR "%s: cannot allocate MURAM for TX FIFO\n",
__func__);
uccf->ucc_fast_tx_virtual_fifo_base_offset = 0;
- ucc_fast_free(uccf);
- return -ENOMEM;
+ goto free_nomem;
}
/* Allocate memory for Rx Virtual Fifo */
@@ -282,8 +281,7 @@ int ucc_fast_init(struct ucc_fast_info * uf_info, struct
ucc_fast_private ** ucc
printk(KERN_ERR "%s: cannot allocate MURAM for RX FIFO\n",
__func__);
uccf->ucc_fast_rx_virtual_fifo_base_offset = 0;
- ucc_fast_free(uccf);
- return -ENOMEM;
+ goto free_nomem;
}
/* Set Virtual Fifo registers */
@@ -312,8 +310,7 @@ int ucc_fast_init(struct ucc_fast_info * uf_info, struct
ucc_fast_private ** ucc
COMM_DIR_RX)) {
printk(KERN_ERR "%s: illegal value for RX clock\n",
__func__);
- ucc_fast_free(uccf);
- return -EINVAL;
+ goto free_inval;
}
/* Tx clock routing */
if ((uf_info->tx_clock != QE_CLK_NONE) &&
@@ -321,8 +318,7 @@ int ucc_fast_init(struct ucc_fast_info * uf_info, struct
ucc_fast_private ** ucc
COMM_DIR_TX)) {
printk(KERN_ERR "%s: illegal value for TX clock\n",
__func__);
- ucc_fast_free(uccf);
- return -EINVAL;
+ goto free_inval;
}
} else {
/* tdm Rx clock routing */
@@ -330,8 +326,7 @@ int ucc_fast_init(struct ucc_fast_info * uf_info, struct
ucc_fast_private ** ucc
ucc_set_tdm_rxtx_clk(uf_info->tdm_num, uf_info->rx_clock,
COMM_DIR_RX)) {
pr_err("%s: illegal value for RX clock", __func__);
- ucc_fast_free(uccf);
- return -EINVAL;
+ goto free_inval;
}
/* tdm Tx clock routing */
@@ -339,8 +334,7 @@ int ucc_fast_init(struct ucc_fast_info * uf_info, struct
ucc_fast_private ** ucc
ucc_set_tdm_rxtx_clk(uf_info->tdm_num, uf_info->tx_clock,
COMM_DIR_TX)) {
pr_err("%s: illegal value for TX clock", __func__);
- ucc_fast_free(uccf);
- return -EINVAL;
+ goto free_inval;
}
/* tdm Rx sync clock routing */
@@ -348,8 +342,7 @@ int ucc_fast_init(struct ucc_fast_info * uf_info, struct
ucc_fast_private ** ucc
ucc_set_tdm_rxtx_sync(uf_info->tdm_num, uf_info->rx_sync,
COMM_DIR_RX)) {
pr_err("%s: illegal value for RX clock", __func__);
- ucc_fast_free(uccf);
- return -EINVAL;
+ goto free_inval;
}
/* tdm Tx sync clock routing */
@@ -357,8 +350,7 @@ int ucc_fast_init(struct ucc_fast_info * uf_info, struct
ucc_fast_private ** ucc
ucc_set_tdm_rxtx_sync(uf_info->tdm_num, uf_info->tx_sync,
COMM_DIR_TX)) {
pr_err("%s: illegal value for TX clock", __func__);
- ucc_fast_free(uccf);
- return -EINVAL;
+ goto free_inval;
}
}
@@ -374,6 +366,14 @@ int ucc_fast_init(struct ucc_fast_info * uf_info, struct
ucc_fast_private ** ucc
*uccf_ret = uccf;
return 0;
+
+free_nomem:
+ ucc_fast_free(uccf);
+ return -ENOMEM;
+
+free_inval:
+ ucc_fast_free(uccf);
+ return -EINVAL;
}
EXPORT_SYMBOL(ucc_fast_init);
--
2.15.1
[PATCH 2/2] ps3: Improve a size determination in five functions
From: Markus Elfring
Date: Sat, 16 Dec 2017 14:21:04 +0100
Replace the specification of data structures by variable references
as the parameter for the operator "sizeof" to make the corresponding size
determination a bit safer according to the Linux coding style convention.
Signed-off-by: Markus Elfring
---
drivers/ps3/ps3-sys-manager.c | 9 ++---
drivers/ps3/ps3-vuart.c | 11 +++
2 files changed, 5 insertions(+), 15 deletions(-)
diff --git a/drivers/ps3/ps3-sys-manager.c b/drivers/ps3/ps3-sys-manager.c
index 73e496a72113..e7d8ef93576a 100644
--- a/drivers/ps3/ps3-sys-manager.c
+++ b/drivers/ps3/ps3-sys-manager.c
@@ -249,9 +249,7 @@ static int ps3_sys_manager_write(struct
ps3_system_bus_device *dev,
BUG_ON(header->payload_size != 8 && header->payload_size != 16);
BUG_ON(header->service_id > 8);
- result = ps3_vuart_write(dev, header,
- sizeof(struct ps3_sys_manager_header));
-
+ result = ps3_vuart_write(dev, header, sizeof(*header));
if (!result)
result = ps3_vuart_write(dev, payload, header->payload_size);
@@ -537,11 +535,8 @@ static int ps3_sys_manager_handle_cmd(struct
ps3_system_bus_device *dev)
static int ps3_sys_manager_handle_msg(struct ps3_system_bus_device *dev)
{
- int result;
struct ps3_sys_manager_header header;
-
- result = ps3_vuart_read(dev, &header,
- sizeof(struct ps3_sys_manager_header));
+ int result = ps3_vuart_read(dev, &header, sizeof(header));
if (result)
return result;
diff --git a/drivers/ps3/ps3-vuart.c b/drivers/ps3/ps3-vuart.c
index bbaad30a876f..c82362cbaf4a 100644
--- a/drivers/ps3/ps3-vuart.c
+++ b/drivers/ps3/ps3-vuart.c
@@ -522,8 +522,7 @@ int ps3_vuart_write(struct ps3_system_bus_device *dev,
const void *buf,
} else
spin_unlock_irqrestore(&priv->tx_list.lock, flags);
- lb = kmalloc(sizeof(struct list_buffer) + bytes, GFP_KERNEL);
-
+ lb = kmalloc(sizeof(*lb) + bytes, GFP_KERNEL);
if (!lb)
return -ENOMEM;
@@ -575,9 +574,7 @@ static int ps3_vuart_queue_rx_bytes(struct
ps3_system_bus_device *dev,
/* Add some extra space for recently arrived data. */
bytes += 128;
-
- lb = kmalloc(sizeof(struct list_buffer) + bytes, GFP_ATOMIC);
-
+ lb = kmalloc(sizeof(*lb) + bytes, GFP_ATOMIC);
if (!lb)
return -ENOMEM;
@@ -925,9 +922,7 @@ static int ps3_vuart_bus_interrupt_get(void)
return 0;
BUG_ON(vuart_bus_priv.bmp);
-
- vuart_bus_priv.bmp = kzalloc(sizeof(struct ports_bmp), GFP_KERNEL);
-
+ vuart_bus_priv.bmp = kzalloc(sizeof(*vuart_bus_priv.bmp), GFP_KERNEL);
if (!vuart_bus_priv.bmp) {
result = -ENOMEM;
goto fail_bmp_malloc;
--
2.15.1
[PATCH 0/2] PS3: Adjustments for six function implementations
From: Markus Elfring Date: Sat, 16 Dec 2017 14:42:24 +0100 Two update suggestions were taken into account from static source code analysis. Markus Elfring (2): Delete an error message for a failed memory allocation in two functions Improve a size determination in five functions drivers/ps3/ps3-lpm.c | 2 -- drivers/ps3/ps3-sys-manager.c | 9 ++--- drivers/ps3/ps3-vuart.c | 12 +++- 3 files changed, 5 insertions(+), 18 deletions(-) -- 2.15.1
[PATCH 1/2] ps3: Delete an error message for a failed memory allocation in two functions
From: Markus Elfring
Date: Sat, 16 Dec 2017 12:32:42 +0100
Omit an extra message for a memory allocation failure in these functions.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring
---
drivers/ps3/ps3-lpm.c | 2 --
drivers/ps3/ps3-vuart.c | 1 -
2 files changed, 3 deletions(-)
diff --git a/drivers/ps3/ps3-lpm.c b/drivers/ps3/ps3-lpm.c
index e34de9a7d517..ac8d5725c9b4 100644
--- a/drivers/ps3/ps3-lpm.c
+++ b/drivers/ps3/ps3-lpm.c
@@ -1123,8 +1123,6 @@ int ps3_lpm_open(enum ps3_lpm_tb_type tb_type, void
*tb_cache,
lpm_priv->tb_cache_internal = kzalloc(
lpm_priv->tb_cache_size + 127, GFP_KERNEL);
if (!lpm_priv->tb_cache_internal) {
- dev_err(sbd_core(), "%s:%u: alloc internal tb_cache "
- "failed\n", __func__, __LINE__);
result = -ENOMEM;
goto fail_malloc;
}
diff --git a/drivers/ps3/ps3-vuart.c b/drivers/ps3/ps3-vuart.c
index b7f300b79ffd..bbaad30a876f 100644
--- a/drivers/ps3/ps3-vuart.c
+++ b/drivers/ps3/ps3-vuart.c
@@ -929,7 +929,6 @@ static int ps3_vuart_bus_interrupt_get(void)
vuart_bus_priv.bmp = kzalloc(sizeof(struct ports_bmp), GFP_KERNEL);
if (!vuart_bus_priv.bmp) {
- pr_debug("%s:%d: kzalloc failed.\n", __func__, __LINE__);
result = -ENOMEM;
goto fail_bmp_malloc;
}
--
2.15.1
Re: ps3: Improve a size determination in five functions
>> Replace the specification of data structures by variable references >> as the parameter for the operator "sizeof" to make the corresponding size >> determination a bit safer according to the Linux coding style convention. > > This would be OK, Thanks. > but you are also removing empty lines and changing the coding format. * Why do you not like such a change combination? * Do you really want to integrate further update steps in this case? Regards, Markus
[PATCH 0/8] PowerPC-NVRAM: Fine-tuning for some function implementations
From: Markus Elfring Date: Thu, 19 Jan 2017 17:41:23 +0100 Some update suggestions were taken into account from static source code analysis. Markus Elfring (8): Return directly after a failed parameter validation in dev_nvram_write() Return directly after a failed kmalloc() in dev_nvram_write() Move an assignment for the variable "ret" in dev_nvram_write() Return directly after a failed parameter validation in dev_nvram_read() Return directly after a failed kmalloc() in dev_nvram_read() Delete three error messages for a failed memory allocation Improve size determinations in three functions Move an assignment for the variable "err" in nvram_scan_partitions() arch/powerpc/kernel/nvram_64.c | 63 +++--- 1 file changed, 22 insertions(+), 41 deletions(-) -- 2.11.0
[PATCH 6/8] powerpc/nvram: Delete three error messages for a failed memory allocation
From: Markus Elfring
Date: Thu, 19 Jan 2017 16:56:46 +0100
The script "checkpatch.pl" pointed information out like the following.
WARNING: Possible unnecessary 'out of memory' message
Thus fix affected source code places.
Signed-off-by: Markus Elfring
---
arch/powerpc/kernel/nvram_64.c | 12 +++-
1 file changed, 3 insertions(+), 9 deletions(-)
diff --git a/arch/powerpc/kernel/nvram_64.c b/arch/powerpc/kernel/nvram_64.c
index 68b970bcf2fc..7af1baaaf01b 100644
--- a/arch/powerpc/kernel/nvram_64.c
+++ b/arch/powerpc/kernel/nvram_64.c
@@ -1040,10 +1040,8 @@ loff_t __init nvram_create_partition(const char *name,
int sig,
/* Create our OS partition */
new_part = kmalloc(sizeof(*new_part), GFP_KERNEL);
- if (!new_part) {
- pr_err("%s: kmalloc failed\n", __func__);
+ if (!new_part)
return -ENOMEM;
- }
new_part->index = free_part->index;
new_part->header.signature = sig;
@@ -1145,10 +1143,8 @@ int __init nvram_scan_partitions(void)
total_size = ppc_md.nvram_size();
header = kmalloc(NVRAM_HEADER_LEN, GFP_KERNEL);
- if (!header) {
- printk(KERN_ERR "nvram_scan_partitions: Failed kmalloc\n");
+ if (!header)
return -ENOMEM;
- }
while (cur_index < total_size) {
@@ -1181,10 +1177,8 @@ int __init nvram_scan_partitions(void)
}
tmp_part = kmalloc(sizeof(struct nvram_partition), GFP_KERNEL);
err = -ENOMEM;
- if (!tmp_part) {
- printk(KERN_ERR "nvram_scan_partitions: kmalloc
failed\n");
+ if (!tmp_part)
goto out;
- }
memcpy(&tmp_part->header, &phead, NVRAM_HEADER_LEN);
tmp_part->index = cur_index;
--
2.11.0
[PATCH 2/8] powerpc/nvram: Return directly after a failed kmalloc() in dev_nvram_write()
From: Markus Elfring Date: Thu, 19 Jan 2017 15:44:03 +0100 Return directly after a call of the function "kmalloc" failed here. Signed-off-by: Markus Elfring --- arch/powerpc/kernel/nvram_64.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/arch/powerpc/kernel/nvram_64.c b/arch/powerpc/kernel/nvram_64.c index 37d08b95c3f0..cf839adf3aa7 100644 --- a/arch/powerpc/kernel/nvram_64.c +++ b/arch/powerpc/kernel/nvram_64.c @@ -802,11 +802,9 @@ static ssize_t dev_nvram_write(struct file *file, const char __user *buf, count = min_t(size_t, count, size - *ppos); count = min(count, PAGE_SIZE); - - ret = -ENOMEM; tmp = kmalloc(count, GFP_KERNEL); if (!tmp) - goto out; + return -ENOMEM; ret = -EFAULT; if (copy_from_user(tmp, buf, count)) -- 2.11.0
[PATCH 4/8] powerpc/nvram: Return directly after a failed parameter validation in dev_nvram_read()
From: Markus Elfring
Date: Thu, 19 Jan 2017 16:12:48 +0100
* Return directly after an inappropriate input parameter was detected.
* Delete an initialisation for the variable "tmp" at the beginning
which became unnecessary with this refactoring.
Signed-off-by: Markus Elfring
---
arch/powerpc/kernel/nvram_64.c | 20 +++-
1 file changed, 7 insertions(+), 13 deletions(-)
diff --git a/arch/powerpc/kernel/nvram_64.c b/arch/powerpc/kernel/nvram_64.c
index dc90a0e9ad65..463551589b97 100644
--- a/arch/powerpc/kernel/nvram_64.c
+++ b/arch/powerpc/kernel/nvram_64.c
@@ -745,24 +745,18 @@ static ssize_t dev_nvram_read(struct file *file, char
__user *buf,
size_t count, loff_t *ppos)
{
ssize_t ret;
- char *tmp = NULL;
+ char *tmp;
ssize_t size;
- if (!ppc_md.nvram_size) {
- ret = -ENODEV;
- goto out;
- }
+ if (!ppc_md.nvram_size)
+ return -ENODEV;
size = ppc_md.nvram_size();
- if (size < 0) {
- ret = size;
- goto out;
- }
+ if (size < 0)
+ return size;
- if (*ppos >= size) {
- ret = 0;
- goto out;
- }
+ if (*ppos >= size)
+ return 0;
count = min_t(size_t, count, size - *ppos);
count = min(count, PAGE_SIZE);
--
2.11.0
[PATCH 3/8] powerpc/nvram: Move an assignment for the variable "ret" in dev_nvram_write()
From: Markus Elfring
Date: Thu, 19 Jan 2017 15:55:36 +0100
A local variable was set to an error code before a concrete error situation
was detected. Thus move the corresponding assignment into an if branch
to indicate a software failure there.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring
---
arch/powerpc/kernel/nvram_64.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/kernel/nvram_64.c b/arch/powerpc/kernel/nvram_64.c
index cf839adf3aa7..dc90a0e9ad65 100644
--- a/arch/powerpc/kernel/nvram_64.c
+++ b/arch/powerpc/kernel/nvram_64.c
@@ -806,9 +806,10 @@ static ssize_t dev_nvram_write(struct file *file, const
char __user *buf,
if (!tmp)
return -ENOMEM;
- ret = -EFAULT;
- if (copy_from_user(tmp, buf, count))
+ if (copy_from_user(tmp, buf, count)) {
+ ret = -EFAULT;
goto out;
+ }
ret = ppc_md.nvram_write(tmp, count, ppos);
--
2.11.0
[PATCH 7/8] powerpc/nvram: Improve size determinations in three functions
From: Markus Elfring
Date: Thu, 19 Jan 2017 17:15:30 +0100
Replace the specification of data structures by references for local
variables as the parameter for the operator "sizeof" to make
the corresponding size determination a bit safer.
Signed-off-by: Markus Elfring
---
arch/powerpc/kernel/nvram_64.c | 9 +++--
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/arch/powerpc/kernel/nvram_64.c b/arch/powerpc/kernel/nvram_64.c
index 7af1baaaf01b..ed54147e3c60 100644
--- a/arch/powerpc/kernel/nvram_64.c
+++ b/arch/powerpc/kernel/nvram_64.c
@@ -207,8 +207,7 @@ int nvram_write_os_partition(struct nvram_os_partition
*part,
tmp_index = part->index;
- rc = ppc_md.nvram_write((char *)&info, sizeof(struct err_log_info),
- &tmp_index);
+ rc = ppc_md.nvram_write((char *)&info, sizeof(info), &tmp_index);
if (rc <= 0) {
pr_err("%s: Failed nvram_write (%d)\n", __func__, rc);
return rc;
@@ -244,9 +243,7 @@ int nvram_read_partition(struct nvram_os_partition *part,
char *buff,
tmp_index = part->index;
if (part->os_partition) {
- rc = ppc_md.nvram_read((char *)&info,
- sizeof(struct err_log_info),
- &tmp_index);
+ rc = ppc_md.nvram_read((char *)&info, sizeof(info), &tmp_index);
if (rc <= 0) {
pr_err("%s: Failed nvram_read (%d)\n", __func__, rc);
return rc;
@@ -1175,7 +1172,7 @@ int __init nvram_scan_partitions(void)
"detected: 0-length partition\n");
goto out;
}
- tmp_part = kmalloc(sizeof(struct nvram_partition), GFP_KERNEL);
+ tmp_part = kmalloc(sizeof(*tmp_part), GFP_KERNEL);
err = -ENOMEM;
if (!tmp_part)
goto out;
--
2.11.0
[PATCH 8/8] powerpc/nvram: Move an assignment for the variable "err" in nvram_scan_partitions()
From: Markus Elfring
Date: Thu, 19 Jan 2017 17:27:37 +0100
A local variable was set to an error code before a concrete error situation
was detected. Thus move the corresponding assignment into an if branch
to indicate a software failure there.
Signed-off-by: Markus Elfring
---
arch/powerpc/kernel/nvram_64.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/kernel/nvram_64.c b/arch/powerpc/kernel/nvram_64.c
index ed54147e3c60..5172115c4ef1 100644
--- a/arch/powerpc/kernel/nvram_64.c
+++ b/arch/powerpc/kernel/nvram_64.c
@@ -1173,9 +1173,10 @@ int __init nvram_scan_partitions(void)
goto out;
}
tmp_part = kmalloc(sizeof(*tmp_part), GFP_KERNEL);
- err = -ENOMEM;
- if (!tmp_part)
+ if (!tmp_part) {
+ err = -ENOMEM;
goto out;
+ }
memcpy(&tmp_part->header, &phead, NVRAM_HEADER_LEN);
tmp_part->index = cur_index;
--
2.11.0
[PATCH 5/8] powerpc/nvram: Return directly after a failed kmalloc() in dev_nvram_read()
From: Markus Elfring
Date: Thu, 19 Jan 2017 16:50:31 +0100
Return directly after a call of the function "kmalloc" failed here.
Signed-off-by: Markus Elfring
---
arch/powerpc/kernel/nvram_64.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/kernel/nvram_64.c b/arch/powerpc/kernel/nvram_64.c
index 463551589b97..68b970bcf2fc 100644
--- a/arch/powerpc/kernel/nvram_64.c
+++ b/arch/powerpc/kernel/nvram_64.c
@@ -762,10 +762,8 @@ static ssize_t dev_nvram_read(struct file *file, char
__user *buf,
count = min(count, PAGE_SIZE);
tmp = kmalloc(count, GFP_KERNEL);
- if (!tmp) {
- ret = -ENOMEM;
- goto out;
- }
+ if (!tmp)
+ return -ENOMEM;
ret = ppc_md.nvram_read(tmp, count, ppos);
if (ret <= 0)
--
2.11.0
[PATCH 1/8] powerpc/nvram: Return directly after a failed parameter validation in dev_nvram_write()
From: Markus Elfring
Date: Thu, 19 Jan 2017 15:22:56 +0100
* Return directly after an inappropriate input parameter was detected.
* Delete an initialisation for the variable "tmp" at the beginning
and an assignment for the variable "ret" which became unnecessary
with this refactoring.
Signed-off-by: Markus Elfring
---
arch/powerpc/kernel/nvram_64.c | 8 +++-
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/kernel/nvram_64.c b/arch/powerpc/kernel/nvram_64.c
index 34d2c595de23..37d08b95c3f0 100644
--- a/arch/powerpc/kernel/nvram_64.c
+++ b/arch/powerpc/kernel/nvram_64.c
@@ -790,17 +790,15 @@ static ssize_t dev_nvram_write(struct file *file, const
char __user *buf,
size_t count, loff_t *ppos)
{
ssize_t ret;
- char *tmp = NULL;
+ char *tmp;
ssize_t size;
- ret = -ENODEV;
if (!ppc_md.nvram_size)
- goto out;
+ return -ENODEV;
- ret = 0;
size = ppc_md.nvram_size();
if (*ppos >= size || size < 0)
- goto out;
+ return 0;
count = min_t(size_t, count, size - *ppos);
count = min(count, PAGE_SIZE);
--
2.11.0
[PATCH] powerpc/rtas_flash: Move an assignment for the variable "rc" in manage_flash_write()
From: Markus Elfring
Date: Thu, 19 Jan 2017 21:20:09 +0100
A local variable was set to an error code before a concrete error situation
was detected. Thus move the corresponding assignment into an if branch
to indicate a software failure there.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring
---
arch/powerpc/kernel/rtas_flash.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/kernel/rtas_flash.c b/arch/powerpc/kernel/rtas_flash.c
index db2b482af658..663904beff67 100644
--- a/arch/powerpc/kernel/rtas_flash.c
+++ b/arch/powerpc/kernel/rtas_flash.c
@@ -419,9 +419,10 @@ static ssize_t manage_flash_write(struct file *file, const
char __user *buf,
op = -1;
if (buf) {
if (count > 9) count = 9;
- rc = -EFAULT;
- if (copy_from_user (stkbuf, buf, count))
+ if (copy_from_user(stkbuf, buf, count)) {
+ rc = -EFAULT;
goto error;
+ }
if (strncmp(stkbuf, reject_str, strlen(reject_str)) == 0)
op = RTAS_REJECT_TMP_IMG;
else if (strncmp(stkbuf, commit_str, strlen(commit_str)) == 0)
--
2.11.0
Re: powerpc/nvram: Move an assignment for the variable "ret" in dev_nvram_write()
> I think you really could have squashed patches 1-3 into a single patch > that returns directly after any failure. Thanks for your constructive feedback. I have got software development concerns around such patch squashing. > At this point you might as well remove that label and move the kfree(tmp) > call up > and return directly after the failure and at the nvram_write() call site > doing away completely with the "ret" variable. Your idea might look nice at first glance. But I would interpret the previous implementation of the discussed function in the way that the memory which was dynamically allocated here should always (not only in the failure case) be released before returning here. Would you really like to change the life time for this “temporary” data item? Regards, Markus
[PATCH 00/11] PowerPC-KVM: Fine-tuning for some function implementations
From: Markus Elfring Date: Fri, 20 Jan 2017 19:07:21 +0100 Some update suggestions were taken into account from static source code analysis. Markus Elfring (11): Move assignments for the variable "err" in kvm_htab_write() Improve a size determination in kvmppc_alloc_hpt() Move error code assignments in two functions Use common error handling code in kvmppc_clr_passthru_irq() Adjust nine checks for null pointers Use kcalloc() in kvmppc_alloc_host_rm_ops() Improve size determinations in five functions Use seq_puts() in xics_debug_show() Improve a size determination in two functions Use kcalloc() in e500_mmu_host_init() Return directly after a failed copy_from_user() in two functions arch/powerpc/kvm/book3s_64_mmu_hv.c | 26 +++-- arch/powerpc/kvm/book3s_hv.c| 73 +++-- arch/powerpc/kvm/book3s_xics.c | 7 ++-- arch/powerpc/kvm/e500_mmu_host.c| 5 +-- arch/powerpc/kvm/powerpc.c | 48 ++-- 5 files changed, 75 insertions(+), 84 deletions(-) -- 2.11.0
[PATCH 01/11] KVM: PPC: Book3S HV: Move assignments for the variable "err" in kvm_htab_write()
From: Markus Elfring
Date: Thu, 19 Jan 2017 22:45:56 +0100
* A local variable was set to an error code in five cases
before a concrete error situation was detected.
Thus move the corresponding assignments into
if branches to indicate a software failure there.
This issue was detected by using the Coccinelle software.
* Delete two zero assignments which became unnecessary with
this refactoring.
Signed-off-by: Markus Elfring
---
arch/powerpc/kvm/book3s_64_mmu_hv.c | 24 +---
1 file changed, 13 insertions(+), 11 deletions(-)
diff --git a/arch/powerpc/kvm/book3s_64_mmu_hv.c
b/arch/powerpc/kvm/book3s_64_mmu_hv.c
index b795dd1ac2ef..dc34729e4ad0 100644
--- a/arch/powerpc/kvm/book3s_64_mmu_hv.c
+++ b/arch/powerpc/kvm/book3s_64_mmu_hv.c
@@ -1399,22 +1399,23 @@ static ssize_t kvm_htab_write(struct file *file, const
char __user *buf,
err = 0;
for (nb = 0; nb + sizeof(hdr) <= count; ) {
- err = -EFAULT;
- if (__copy_from_user(&hdr, buf, sizeof(hdr)))
+ if (__copy_from_user(&hdr, buf, sizeof(hdr))) {
+ err = -EFAULT;
break;
+ }
- err = 0;
if (nb + hdr.n_valid * HPTE_SIZE > count)
break;
nb += sizeof(hdr);
buf += sizeof(hdr);
- err = -EINVAL;
i = hdr.index;
if (i >= kvm->arch.hpt_npte ||
- i + hdr.n_valid + hdr.n_invalid > kvm->arch.hpt_npte)
+ i + hdr.n_valid + hdr.n_invalid > kvm->arch.hpt_npte) {
+ err = -EINVAL;
break;
+ }
hptp = (__be64 *)(kvm->arch.hpt_virt + (i * HPTE_SIZE));
lbuf = (unsigned long __user *)buf;
@@ -1422,26 +1423,28 @@ static ssize_t kvm_htab_write(struct file *file, const
char __user *buf,
__be64 hpte_v;
__be64 hpte_r;
- err = -EFAULT;
if (__get_user(hpte_v, lbuf) ||
- __get_user(hpte_r, lbuf + 1))
+ __get_user(hpte_r, lbuf + 1)) {
+ err = -EFAULT;
goto out;
+ }
v = be64_to_cpu(hpte_v);
r = be64_to_cpu(hpte_r);
- err = -EINVAL;
- if (!(v & HPTE_V_VALID))
+ if (!(v & HPTE_V_VALID)) {
+ err = -EINVAL;
goto out;
+ }
lbuf += 2;
nb += HPTE_SIZE;
if (be64_to_cpu(hptp[0]) & (HPTE_V_VALID |
HPTE_V_ABSENT))
kvmppc_do_h_remove(kvm, 0, i, 0, tmp);
- err = -EIO;
ret = kvmppc_virtmode_do_h_enter(kvm, H_EXACT, i, v, r,
tmp);
if (ret != H_SUCCESS) {
pr_err("kvm_htab_write ret %ld i=%ld v=%lx "
"r=%lx\n", ret, i, v, r);
+ err = -EIO;
goto out;
}
if (!hpte_setup && is_vrma_hpte(v)) {
@@ -1465,7 +1468,6 @@ static ssize_t kvm_htab_write(struct file *file, const
char __user *buf,
++i;
hptp += 2;
}
- err = 0;
}
out:
--
2.11.0
[PATCH 02/11] KVM: PPC: Book3S HV: Improve a size determination in kvmppc_alloc_hpt()
From: Markus Elfring
Date: Fri, 20 Jan 2017 10:00:13 +0100
Replace the specification of a data structure by a pointer dereference
as the parameter for the operator "sizeof" to make the corresponding size
determination a bit safer according to the Linux coding style convention.
Signed-off-by: Markus Elfring
---
arch/powerpc/kvm/book3s_64_mmu_hv.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/kvm/book3s_64_mmu_hv.c
b/arch/powerpc/kvm/book3s_64_mmu_hv.c
index dc34729e4ad0..4655f27e0518 100644
--- a/arch/powerpc/kvm/book3s_64_mmu_hv.c
+++ b/arch/powerpc/kvm/book3s_64_mmu_hv.c
@@ -91,7 +91,7 @@ long kvmppc_alloc_hpt(struct kvm *kvm, u32 *htab_orderp)
atomic64_set(&kvm->arch.mmio_update, 0);
/* Allocate reverse map array */
- rev = vmalloc(sizeof(struct revmap_entry) * kvm->arch.hpt_npte);
+ rev = vmalloc(sizeof(*rev) * kvm->arch.hpt_npte);
if (!rev) {
pr_err("kvmppc_alloc_hpt: Couldn't alloc reverse map array\n");
goto out_freehpt;
--
2.11.0
[PATCH 03/11] KVM: PPC: Book3S HV: Move error code assignments in two functions
From: Markus Elfring
Date: Fri, 20 Jan 2017 10:30:26 +0100
A local variable was set to an error code in a few cases before a concrete
error situation was detected. Thus move the corresponding assignments into
if branches to indicate a software failure there.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring
---
arch/powerpc/kvm/book3s_hv.c | 25 +++--
1 file changed, 15 insertions(+), 10 deletions(-)
diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index 8dcbe37a4dac..a93e1c4445da 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -2967,15 +2967,17 @@ static int kvm_vm_ioctl_get_dirty_log_hv(struct kvm
*kvm,
mutex_lock(&kvm->slots_lock);
- r = -EINVAL;
- if (log->slot >= KVM_USER_MEM_SLOTS)
+ if (log->slot >= KVM_USER_MEM_SLOTS) {
+ r = -EINVAL;
goto out;
+ }
slots = kvm_memslots(kvm);
memslot = id_to_memslot(slots, log->slot);
- r = -ENOENT;
- if (!memslot->dirty_bitmap)
+ if (!memslot->dirty_bitmap) {
+ r = -ENOENT;
goto out;
+ }
n = kvm_dirty_bitmap_bytes(memslot);
memset(memslot->dirty_bitmap, 0, n);
@@ -2984,9 +2986,10 @@ static int kvm_vm_ioctl_get_dirty_log_hv(struct kvm *kvm,
if (r)
goto out;
- r = -EFAULT;
- if (copy_to_user(log->dirty_bitmap, memslot->dirty_bitmap, n))
+ if (copy_to_user(log->dirty_bitmap, memslot->dirty_bitmap, n)) {
+ r = -EFAULT;
goto out;
+ }
r = 0;
out:
@@ -3127,9 +3130,10 @@ static int kvmppc_hv_setup_htab_rma(struct kvm_vcpu
*vcpu)
memslot = gfn_to_memslot(kvm, 0);
/* We must have some memory at 0 by now */
- err = -EINVAL;
- if (!memslot || (memslot->flags & KVM_MEMSLOT_INVALID))
+ if (!memslot || (memslot->flags & KVM_MEMSLOT_INVALID)) {
+ err = -EINVAL;
goto out_srcu;
+ }
/* Look up the VMA for the start of this memory slot */
hva = memslot->userspace_addr;
@@ -3144,10 +3148,11 @@ static int kvmppc_hv_setup_htab_rma(struct kvm_vcpu
*vcpu)
up_read(¤t->mm->mmap_sem);
/* We can handle 4k, 64k or 16M pages in the VRMA */
- err = -EINVAL;
if (!(psize == 0x1000 || psize == 0x1 ||
- psize == 0x100))
+ psize == 0x100)) {
+ err = -EINVAL;
goto out_srcu;
+ }
senc = slb_pgsize_encoding(psize);
kvm->arch.vrma_slb_v = senc | SLB_VSID_B_1T |
--
2.11.0
[PATCH 04/11] KVM: PPC: Book3S HV: Use common error handling code in kvmppc_clr_passthru_irq()
From: Markus Elfring
Date: Fri, 20 Jan 2017 11:00:08 +0100
Add a jump target so that a bit of exception handling can be better reused
at the end of this function.
Signed-off-by: Markus Elfring
---
arch/powerpc/kvm/book3s_hv.c | 8 +++-
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index a93e1c4445da..cfc7699d05df 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -3509,11 +3509,9 @@ static int kvmppc_clr_passthru_irq(struct kvm *kvm, int
host_irq, int guest_gsi)
return -EIO;
mutex_lock(&kvm->lock);
+ if (!kvm->arch.pimap)
+ goto unlock;
- if (kvm->arch.pimap == NULL) {
- mutex_unlock(&kvm->lock);
- return 0;
- }
pimap = kvm->arch.pimap;
for (i = 0; i < pimap->n_mapped; i++) {
@@ -3535,7 +3533,7 @@ static int kvmppc_clr_passthru_irq(struct kvm *kvm, int
host_irq, int guest_gsi)
* We don't free this structure even when the count goes to
* zero. The structure is freed when we destroy the VM.
*/
-
+unlock:
mutex_unlock(&kvm->lock);
return 0;
}
--
2.11.0
