Re: [PATCH -next v2 1/3] mm: treewide: remove unused address argument from pte_alloc functions

[SF Markus Elfring]

> … 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

Re: [PATCH v2 1/2] treewide: remove unused address argument from pte_alloc functions

[SF Markus Elfring]

>>> 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 v2 1/2] treewide: remove unused address argument from pte_alloc functions

[SF Markus Elfring]

> 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 RFC 3/5] coccinelle: add xxxsetbitsXX converting spatch

[SF Markus Elfring]

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

[PATCH] powerpc: Use common error handling code in setup_new_fdt()

[SF Markus Elfring]

From: Markus Elfring 
Date: Sun, 11 Mar 2018 09:03:42 +0100

Add a jump target so that a bit of exception handling can be better reused
at the end of this function.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring 
---
 arch/powerpc/kernel/machine_kexec_file_64.c | 28 
 1 file changed, 12 insertions(+), 16 deletions(-)

diff --git a/arch/powerpc/kernel/machine_kexec_file_64.c 
b/arch/powerpc/kernel/machine_kexec_file_64.c
index e4395f937d63..90c6004c2eec 100644
--- a/arch/powerpc/kernel/machine_kexec_file_64.c
+++ b/arch/powerpc/kernel/machine_kexec_file_64.c
@@ -302,18 +302,14 @@ int setup_new_fdt(const struct kimage *image, void *fdt,
ret = fdt_setprop_u64(fdt, chosen_node,
  "linux,initrd-start",
  initrd_load_addr);
-   if (ret < 0) {
-   pr_err("Error setting up the new device tree.\n");
-   return -EINVAL;
-   }
+   if (ret < 0)
+   goto report_setup_failure;
 
/* initrd-end is the first address after the initrd image. */
ret = fdt_setprop_u64(fdt, chosen_node, "linux,initrd-end",
  initrd_load_addr + initrd_len);
-   if (ret < 0) {
-   pr_err("Error setting up the new device tree.\n");
-   return -EINVAL;
-   }
+   if (ret < 0)
+   goto report_setup_failure;
 
ret = fdt_add_mem_rsv(fdt, initrd_load_addr, initrd_len);
if (ret) {
@@ -325,10 +321,8 @@ int setup_new_fdt(const struct kimage *image, void *fdt,
 
if (cmdline != NULL) {
ret = fdt_setprop_string(fdt, chosen_node, "bootargs", cmdline);
-   if (ret < 0) {
-   pr_err("Error setting up the new device tree.\n");
-   return -EINVAL;
-   }
+   if (ret < 0)
+   goto report_setup_failure;
} else {
ret = fdt_delprop(fdt, chosen_node, "bootargs");
if (ret && ret != -FDT_ERR_NOTFOUND) {
@@ -344,10 +338,12 @@ int setup_new_fdt(const struct kimage *image, void *fdt,
}
 
ret = fdt_setprop(fdt, chosen_node, "linux,booted-from-kexec", NULL, 0);
-   if (ret) {
-   pr_err("Error setting up the new device tree.\n");
-   return -EINVAL;
-   }
+   if (ret)
+   goto report_setup_failure;
 
return 0;
+
+report_setup_failure:
+   pr_err("Error setting up the new device tree.\n");
+   return -EINVAL;
 }
-- 
2.16.2

[PATCH] crypto: nx-842: Delete an error message for a failed memory allocation in nx842_pseries_init()

[SF Markus Elfring]

From: Markus Elfring 
Date: Wed, 14 Feb 2018 17:05:13 +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/crypto/nx/nx-842-pseries.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/crypto/nx/nx-842-pseries.c 
b/drivers/crypto/nx/nx-842-pseries.c
index bf52cd1d7fca..66869976cfa2 100644
--- a/drivers/crypto/nx/nx-842-pseries.c
+++ b/drivers/crypto/nx/nx-842-pseries.c
@@ -1105,10 +1105,9 @@ static int __init nx842_pseries_init(void)
 
RCU_INIT_POINTER(devdata, NULL);
new_devdata = kzalloc(sizeof(*new_devdata), GFP_KERNEL);
-   if (!new_devdata) {
-   pr_err("Could not allocate memory for device data\n");
+   if (!new_devdata)
return -ENOMEM;
-   }
+
RCU_INIT_POINTER(devdata, new_devdata);
 
ret = vio_register_driver(_vio_driver);
-- 
2.16.1

[PATCH 2/2] macintosh/rack-meter: Improve a size determination in rackmeter_probe()

[SF Markus Elfring]

From: Markus Elfring 
Date: Tue, 16 Jan 2018 21:26:52 +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/macintosh/rack-meter.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/macintosh/rack-meter.c b/drivers/macintosh/rack-meter.c
index 0796b74174d4..800fc6822ad8 100644
--- a/drivers/macintosh/rack-meter.c
+++ b/drivers/macintosh/rack-meter.c
@@ -397,7 +397,7 @@ static int rackmeter_probe(struct macio_dev* mdev,
}
 
/* Create and initialize our instance data */
-   rm = kzalloc(sizeof(struct rackmeter), GFP_KERNEL);
+   rm = kzalloc(sizeof(*rm), GFP_KERNEL);
if (rm == NULL) {
rc = -ENOMEM;
goto bail_release;
-- 
2.15.1

[PATCH 1/2] macintosh/rack-meter: Delete an error message for a failed memory allocation in rackmeter_probe()

[SF Markus Elfring]

From: Markus Elfring 
Date: Tue, 16 Jan 2018 21:23:36 +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/macintosh/rack-meter.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/macintosh/rack-meter.c b/drivers/macintosh/rack-meter.c
index 910b5b6f96b1..0796b74174d4 100644
--- a/drivers/macintosh/rack-meter.c
+++ b/drivers/macintosh/rack-meter.c
@@ -399,7 +399,6 @@ static int rackmeter_probe(struct macio_dev* mdev,
/* Create and initialize our instance data */
rm = kzalloc(sizeof(struct rackmeter), GFP_KERNEL);
if (rm == NULL) {
-   printk(KERN_ERR "rackmeter: failed to allocate memory !\n");
rc = -ENOMEM;
goto bail_release;
}
-- 
2.15.1

[PATCH 0/2] macintosh/rack-meter: Adjustments for rackmeter_probe()

[SF Markus Elfring]

From: Markus Elfring 
Date: Tue, 16 Jan 2018 21:36:56 +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
  Improve a size determination

 drivers/macintosh/rack-meter.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

-- 
2.15.1

[PATCH 4/5] powerpc/4xx: Improve a size determination in two functions

[SF Markus Elfring]

From: Markus Elfring 
Date: Thu, 11 Jan 2018 18:32:33 +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 
---
 arch/powerpc/platforms/4xx/msi.c | 2 +-
 arch/powerpc/platforms/4xx/ocm.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/platforms/4xx/msi.c b/arch/powerpc/platforms/4xx/msi.c
index d16b81303cb0..f693cab07752 100644
--- a/arch/powerpc/platforms/4xx/msi.c
+++ b/arch/powerpc/platforms/4xx/msi.c
@@ -223,7 +223,7 @@ static int ppc4xx_msi_probe(struct platform_device *dev)
 
dev_dbg(>dev, "PCIE-MSI: Setting up MSI support...\n");
 
-   msi = kzalloc(sizeof(struct ppc4xx_msi), GFP_KERNEL);
+   msi = kzalloc(sizeof(*msi), GFP_KERNEL);
if (!msi)
return -ENOMEM;
 
diff --git a/arch/powerpc/platforms/4xx/ocm.c b/arch/powerpc/platforms/4xx/ocm.c
index 5a750d0ad446..11418f090bf4 100644
--- a/arch/powerpc/platforms/4xx/ocm.c
+++ b/arch/powerpc/platforms/4xx/ocm.c
@@ -335,7 +335,7 @@ void *ppc4xx_ocm_alloc(phys_addr_t *phys, int size, int 
align,
if (IS_ERR_VALUE(offset))
continue;
 
-   ocm_blk = kzalloc(sizeof(struct ocm_block), GFP_KERNEL);
+   ocm_blk = kzalloc(sizeof(*ocm_blk), GFP_KERNEL);
if (!ocm_blk) {
rh_free(ocm_reg->rh, offset);
break;
-- 
2.15.1

[PATCH 5/5] powerpc/4xx: Delete an unnecessary return statement in two functions

[SF Markus Elfring]

From: Markus Elfring 
Date: Thu, 11 Jan 2018 18:40:23 +0100

The script "checkpatch.pl" pointed information out like the following.

WARNING: void function return statements are not generally useful

Thus remove such a statement in the affected functions.

Signed-off-by: Markus Elfring 
---
 arch/powerpc/platforms/4xx/ocm.c | 2 --
 arch/powerpc/platforms/4xx/pci.c | 1 -
 2 files changed, 3 deletions(-)

diff --git a/arch/powerpc/platforms/4xx/ocm.c b/arch/powerpc/platforms/4xx/ocm.c
index 11418f090bf4..aee5f041ff14 100644
--- a/arch/powerpc/platforms/4xx/ocm.c
+++ b/arch/powerpc/platforms/4xx/ocm.c
@@ -226,8 +226,6 @@ static void __init ocm_init_node(int count, struct 
device_node *node)
INIT_LIST_HEAD(>c.list);
 
ocm->ready = 1;
-
-   return;
 }
 
 static int ocm_debugfs_show(struct seq_file *m, void *v)
diff --git a/arch/powerpc/platforms/4xx/pci.c b/arch/powerpc/platforms/4xx/pci.c
index 73e6b36bcd51..b73ea8f4be1f 100644
--- a/arch/powerpc/platforms/4xx/pci.c
+++ b/arch/powerpc/platforms/4xx/pci.c
@@ -1399,7 +1399,6 @@ static void __init ppc_476fpe_pciex_check_link(struct 
ppc4xx_pciex_port *port)
printk(KERN_WARNING "PCIE%d: Link up failed\n", port->index);
 
iounmap(mbase);
-   return;
 }
 
 static struct ppc4xx_pciex_hwops ppc_476fpe_pcie_hwops __initdata =
-- 
2.15.1

[PATCH 3/5] powerpc/4xx: Delete an error message for a failed memory allocation in three functions

[SF Markus Elfring]

From: Markus Elfring 
Date: Thu, 11 Jan 2018 18:28:54 +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 
---
 arch/powerpc/platforms/4xx/msi.c | 5 ++---
 arch/powerpc/platforms/4xx/ocm.c | 5 +
 2 files changed, 3 insertions(+), 7 deletions(-)

diff --git a/arch/powerpc/platforms/4xx/msi.c b/arch/powerpc/platforms/4xx/msi.c
index d50417e23add..d16b81303cb0 100644
--- a/arch/powerpc/platforms/4xx/msi.c
+++ b/arch/powerpc/platforms/4xx/msi.c
@@ -224,10 +224,9 @@ static int ppc4xx_msi_probe(struct platform_device *dev)
dev_dbg(>dev, "PCIE-MSI: Setting up MSI support...\n");
 
msi = kzalloc(sizeof(struct ppc4xx_msi), GFP_KERNEL);
-   if (!msi) {
-   dev_err(>dev, "No memory for MSI structure\n");
+   if (!msi)
return -ENOMEM;
-   }
+
dev->dev.platform_data = msi;
 
/* Get MSI ranges */
diff --git a/arch/powerpc/platforms/4xx/ocm.c b/arch/powerpc/platforms/4xx/ocm.c
index 8cb601e8bed7..5a750d0ad446 100644
--- a/arch/powerpc/platforms/4xx/ocm.c
+++ b/arch/powerpc/platforms/4xx/ocm.c
@@ -337,7 +337,6 @@ void *ppc4xx_ocm_alloc(phys_addr_t *phys, int size, int 
align,
 
ocm_blk = kzalloc(sizeof(struct ocm_block), GFP_KERNEL);
if (!ocm_blk) {
-   printk(KERN_ERR "PPC4XX OCM: could not allocate ocm 
block");
rh_free(ocm_reg->rh, offset);
break;
}
@@ -391,10 +390,8 @@ static int __init ppc4xx_ocm_init(void)
return 0;
 
ocm_nodes = kzalloc((count * sizeof(struct ocm_info)), GFP_KERNEL);
-   if (!ocm_nodes) {
-   printk(KERN_ERR "PPC4XX OCM: failed to allocate OCM nodes!\n");
+   if (!ocm_nodes)
return -ENOMEM;
-   }
 
ocm_count = count;
count = 0;
-- 
2.15.1

[PATCH 2/5] powerpc/4xx: Use seq_putc() in ocm_debugfs_show()

[SF Markus Elfring]

From: Markus Elfring 
Date: Thu, 11 Jan 2018 18:10:02 +0100

A single character (line break) 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/platforms/4xx/ocm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/platforms/4xx/ocm.c b/arch/powerpc/platforms/4xx/ocm.c
index 5bc1baf809d1..8cb601e8bed7 100644
--- a/arch/powerpc/platforms/4xx/ocm.c
+++ b/arch/powerpc/platforms/4xx/ocm.c
@@ -267,7 +267,7 @@ static int ocm_debugfs_show(struct seq_file *m, void *v)
blk->size, blk->owner);
}
 
-   seq_printf(m, "\n");
+   seq_putc(m, '\n');
}
 
return 0;
-- 
2.15.1

[PATCH 1/5] powerpc/4xx: Combine four seq_printf() calls into two in ocm_debugfs_show()

[SF Markus Elfring]

From: Markus Elfring 
Date: Thu, 11 Jan 2018 18:08:08 +0100

Some data were printed into a sequence by four separate function calls.
Print the same data by two single function calls instead.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring 
---
 arch/powerpc/platforms/4xx/ocm.c | 8 ++--
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/arch/powerpc/platforms/4xx/ocm.c b/arch/powerpc/platforms/4xx/ocm.c
index 85d9e37f5ccb..5bc1baf809d1 100644
--- a/arch/powerpc/platforms/4xx/ocm.c
+++ b/arch/powerpc/platforms/4xx/ocm.c
@@ -245,9 +245,7 @@ static int ocm_debugfs_show(struct seq_file *m, void *v)
seq_printf(m, "PhysAddr : 0x%llx\n", ocm->phys);
seq_printf(m, "MemTotal : %d Bytes\n", ocm->memtotal);
seq_printf(m, "MemTotal(NC) : %d Bytes\n", ocm->nc.memtotal);
-   seq_printf(m, "MemTotal(C)  : %d Bytes\n", ocm->c.memtotal);
-
-   seq_printf(m, "\n");
+   seq_printf(m, "MemTotal(C)  : %d Bytes\n\n", ocm->c.memtotal);
 
seq_printf(m, "NC.PhysAddr  : 0x%llx\n", ocm->nc.phys);
seq_printf(m, "NC.VirtAddr  : 0x%p\n", ocm->nc.virt);
@@ -259,9 +257,7 @@ static int ocm_debugfs_show(struct seq_file *m, void *v)
blk->size, blk->owner);
}
 
-   seq_printf(m, "\n");
-
-   seq_printf(m, "C.PhysAddr   : 0x%llx\n", ocm->c.phys);
+   seq_printf(m, "\nC.PhysAddr   : 0x%llx\n", ocm->c.phys);
seq_printf(m, "C.VirtAddr   : 0x%p\n", ocm->c.virt);
seq_printf(m, "C.MemTotal   : %d Bytes\n", ocm->c.memtotal);
seq_printf(m, "C.MemFree: %d Bytes\n", ocm->c.memfree);
-- 
2.15.1

[PATCH 0/5] powerpc/platforms/4xx: Adjustments for six function implementations

[SF Markus Elfring]

From: Markus Elfring 
Date: Thu, 11 Jan 2018 18:48:28 +0100

A few update suggestions were taken into account
from static source code analysis.

Markus Elfring (5):
  Combine four seq_printf() calls into two in ocm_debugfs_show()
  Use seq_putc() in ocm_debugfs_show()
  Delete an error message for a failed memory allocation in three functions
  Improve a size determination in two functions
  Delete an unnecessary return statement in two functions

 arch/powerpc/platforms/4xx/msi.c |  7 +++
 arch/powerpc/platforms/4xx/ocm.c | 19 +--
 arch/powerpc/platforms/4xx/pci.c |  1 -
 3 files changed, 8 insertions(+), 19 deletions(-)

-- 
2.15.1

[PATCH] KVM: PPC: Use seq_puts() in kvmppc_exit_timing_show()

[SF Markus Elfring]

From: Markus Elfring 
Date: Sun, 7 Jan 2018 10:07:36 +0100

A headline should be quickly put into a sequence. Thus use the
function "seq_puts" instead of "seq_printf" for this purpose.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring 
---
 arch/powerpc/kvm/timing.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/powerpc/kvm/timing.c b/arch/powerpc/kvm/timing.c
index e44d2b2ea97e..1c03c978eb18 100644
--- a/arch/powerpc/kvm/timing.c
+++ b/arch/powerpc/kvm/timing.c
@@ -143,8 +143,7 @@ static int kvmppc_exit_timing_show(struct seq_file *m, void 
*private)
int i;
u64 min, max, sum, sum_quad;
 
-   seq_printf(m, "%s", "type   count   min max sum 
sum_squared\n");
-
+   seq_puts(m, "type   count   min max sum sum_squared\n");
 
for (i = 0; i < __NUMBER_OF_KVM_EXIT_TYPES; i++) {
 
-- 
2.15.1

[PATCH] ps3_gelic_net: Delete an error message for a failed memory allocation in gelic_descr_prepare_rx()

[SF Markus Elfring]

From: Markus Elfring 
Date: Wed, 3 Jan 2018 14:50:59 +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/net/ethernet/toshiba/ps3_gelic_net.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/net/ethernet/toshiba/ps3_gelic_net.c 
b/drivers/net/ethernet/toshiba/ps3_gelic_net.c
index 88d74aef218a..644b875e324c 100644
--- a/drivers/net/ethernet/toshiba/ps3_gelic_net.c
+++ b/drivers/net/ethernet/toshiba/ps3_gelic_net.c
@@ -395,8 +395,6 @@ static int gelic_descr_prepare_rx(struct gelic_card *card,
descr->skb = dev_alloc_skb(bufsize + GELIC_NET_RXBUF_ALIGN - 1);
if (!descr->skb) {
descr->buf_addr = 0; /* tell DMAC don't touch memory */
-   dev_info(ctodev(card),
-"%s:allocate skb failed !!\n", __func__);
return -ENOMEM;
}
descr->buf_size = cpu_to_be32(bufsize);
-- 
2.15.1

[PATCH] wan/fsl_ucc_hdlc: Delete an error message for a failed memory allocation in ucc_hdlc_probe()

[SF Markus Elfring]

From: Markus Elfring 
Date: Sat, 30 Dec 2017 22:25:44 +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/net/wan/fsl_ucc_hdlc.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/net/wan/fsl_ucc_hdlc.c b/drivers/net/wan/fsl_ucc_hdlc.c
index 33df76405b86..98f8be206bae 100644
--- a/drivers/net/wan/fsl_ucc_hdlc.c
+++ b/drivers/net/wan/fsl_ucc_hdlc.c
@@ -1082,7 +1082,6 @@ static int ucc_hdlc_probe(struct platform_device *pdev)
utdm = kzalloc(sizeof(*utdm), GFP_KERNEL);
if (!utdm) {
ret = -ENOMEM;
-   dev_err(>dev, "No mem to alloc ucc tdm data\n");
goto free_uhdlc_priv;
}
uhdlc_priv->utdm = utdm;
-- 
2.15.1

[PATCH] pci/hotplug: Delete error messages for a failed memory allocation in four functions

[SF Markus Elfring]

From: Markus Elfring 
Date: Fri, 29 Dec 2017 15:47:00 +0100

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 
---
 drivers/pci/hotplug/pciehp_hpc.c  | 5 ++---
 drivers/pci/hotplug/pnv_php.c | 8 +---
 drivers/pci/hotplug/shpchp_core.c | 5 ++---
 3 files changed, 5 insertions(+), 13 deletions(-)

diff --git a/drivers/pci/hotplug/pciehp_hpc.c b/drivers/pci/hotplug/pciehp_hpc.c
index 7bab0606f1a9..ed27635ec740 100644
--- a/drivers/pci/hotplug/pciehp_hpc.c
+++ b/drivers/pci/hotplug/pciehp_hpc.c
@@ -838,10 +838,9 @@ struct controller *pcie_init(struct pcie_device *dev)
struct pci_dev *pdev = dev->port;
 
ctrl = kzalloc(sizeof(*ctrl), GFP_KERNEL);
-   if (!ctrl) {
-   dev_err(>device, "%s: Out of memory\n", __func__);
+   if (!ctrl)
goto abort;
-   }
+
ctrl->pcie = dev;
pcie_capability_read_dword(pdev, PCI_EXP_SLTCAP, _cap);
 
diff --git a/drivers/pci/hotplug/pnv_php.c b/drivers/pci/hotplug/pnv_php.c
index 74f6a17e4614..4ab318a0800d 100644
--- a/drivers/pci/hotplug/pnv_php.c
+++ b/drivers/pci/hotplug/pnv_php.c
@@ -266,7 +266,6 @@ static int pnv_php_add_devtree(struct pnv_php_slot 
*php_slot)
fdt1 = kzalloc(0x1, GFP_KERNEL);
if (!fdt1) {
ret = -ENOMEM;
-   dev_warn(_slot->pdev->dev, "Cannot alloc FDT blob\n");
goto out;
}
 
@@ -280,8 +279,6 @@ static int pnv_php_add_devtree(struct pnv_php_slot 
*php_slot)
fdt = kzalloc(fdt_totalsize(fdt1), GFP_KERNEL);
if (!fdt) {
ret = -ENOMEM;
-   dev_warn(_slot->pdev->dev, "Cannot %d bytes memory\n",
-fdt_totalsize(fdt1));
goto free_fdt1;
}
 
@@ -756,11 +753,8 @@ static irqreturn_t pnv_php_interrupt(int irq, void *data)
 * fine as the PCI devices (PE) aren't functional any more.
 */
event = kzalloc(sizeof(*event), GFP_ATOMIC);
-   if (!event) {
-   dev_warn(>dev, "PCI slot [%s] missed hotplug event 
0x%04x\n",
-php_slot->name, sts);
+   if (!event)
return IRQ_HANDLED;
-   }
 
dev_info(>dev, "PCI slot [%s] %s (IRQ: %d)\n",
 php_slot->name, added ? "added" : "removed", irq);
diff --git a/drivers/pci/hotplug/shpchp_core.c 
b/drivers/pci/hotplug/shpchp_core.c
index 7bfb87bd2b7e..2f2b87acf179 100644
--- a/drivers/pci/hotplug/shpchp_core.c
+++ b/drivers/pci/hotplug/shpchp_core.c
@@ -305,10 +305,9 @@ static int shpc_probe(struct pci_dev *pdev, const struct 
pci_device_id *ent)
return -ENODEV;
 
ctrl = kzalloc(sizeof(*ctrl), GFP_KERNEL);
-   if (!ctrl) {
-   dev_err(>dev, "%s: Out of memory\n", __func__);
+   if (!ctrl)
goto err_out_none;
-   }
+
INIT_LIST_HEAD(>slot_list);
 
rc = shpc_init(ctrl, pdev);
-- 
2.15.1

Re: ps3: Improve a size determination in five functions

[SF Markus Elfring]

> Some observations:
> 
>  o Your patch fixes no bug nor replaces any depreciated feature.

How do you think about information from the section “14) Allocating memory”
in the document “coding-style.rst” for the shown source code transformation?


>  o There will be no functional change; …

Yes. - The suggested adjustment should work in this way generally.

Regards,
Markus

Re: ps3: Improve a size determination in five functions

[SF Markus Elfring]

>> 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 1/2] ps3: Delete an error message for a failed memory allocation in two functions

[SF Markus Elfring]

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

[PATCH 0/2] PS3: Adjustments for six function implementations

[SF Markus Elfring]

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 2/2] ps3: Improve a size determination in five functions

[SF Markus Elfring]

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, ,
-   sizeof(struct ps3_sys_manager_header));
+   int result = ps3_vuart_read(dev, , 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(>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 3/3] soc/fsl/qe: Use common error handling code in ucc_fast_init()

[SF Markus Elfring]

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/3] soc/fsl/qe: Improve a size determination in two functions

[SF Markus Elfring]

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 1/3] soc/fsl/qe: Delete an error message for a failed memory allocation in three functions

[SF Markus Elfring]

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 0/3] SoC/FSL/QE: Adjustments for three function implementations

[SF Markus Elfring]

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 3/3] tty/serial/ucc_uart: Fix a typo in a comment line

[SF Markus Elfring]

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 2/3] tty/serial/ucc_uart: Improve a size determination in ucc_uart_probe()

[SF Markus Elfring]

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 1/3] tty/serial/ucc_uart: Delete an error message for a failed memory allocation in ucc_uart_probe()

[SF Markus Elfring]

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(>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, );
-- 
2.15.1

[PATCH 0/3] tty/serial/ucc_uart: Adjustments for two functions

[SF Markus Elfring]

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

Re: char/tpm: Less checks in tpm_ibmvtpm_probe() after error detection

[SF Markus Elfring]

>> 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

Re: Circumstances for using the tag “Fixes” (or not)

[SF Markus Elfring]

>>> 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: [PATCH 4/4] char/tpm: Less checks in tpm_ibmvtpm_probe() after error detection

[SF Markus Elfring]

>> @@ -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: [PATCH 4/5] powerpc-pseries: Return directly after a failed kzalloc_node() in iommu_pseries_alloc_group()

[SF Markus Elfring]

>> @@ -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 3/5] powerpc-pseries: Delete an unnecessary variable initialisation in iommu_pseries_alloc_group()

[SF Markus Elfring]

>>  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

[PATCH 5/5] powerpc-pseries: Less function calls in iommu_pseries_alloc_group() after error detection

[SF Markus Elfring]

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(>it_group_list);
kref_init(>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

[PATCH 4/5] powerpc-pseries: Return directly after a failed kzalloc_node() in iommu_pseries_alloc_group()

[SF Markus Elfring]

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 3/5] powerpc-pseries: Delete an unnecessary variable initialisation in iommu_pseries_alloc_group()

[SF Markus Elfring]

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 2/5] powerpc-pseries: Improve nine size determinations

[SF Markus Elfring]

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 1/5] powerpc-pseries: Delete five error messages for a failed memory allocation

[SF Markus Elfring]

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(>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,
-   "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,
-   "couldn't allocate property name and value\n");
+   if (!win64->name || !win64->value)
goto out_free_prop;
-   }
 
ret = create_ddw(dev, ddw_avail, , 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 0/5] PowerPC-pSeries: Adjustments for seven function implementations

[SF Markus Elfring]

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

Re: char/tpm: Delete an error message for a failed memory allocation in tpm_…()

[SF Markus Elfring]

>> 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

Re: char/tpm: Improve a size determination in nine functions

[SF Markus Elfring]

> 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: Improve a size determination in nine functions

[SF Markus Elfring]

> 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

[SF Markus Elfring]

> 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: Adjustments for ten function implementations

[SF Markus Elfring]

>>> 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

[SF Markus Elfring]

>> 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: Adjusting further size determinations?

[SF Markus Elfring]

>> 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(, 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: Adjusting further size determinations?

[SF Markus Elfring]

 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?

[SF Markus Elfring]

> 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: char-TPM: Adjustments for ten function implementations

[SF Markus Elfring]

>> 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: char-TPM: Adjustments for ten function implementations

[SF Markus Elfring]

> 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: Improve a size determination in nine functions

[SF Markus Elfring]

>> 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: Improve a size determination in nine functions

[SF Markus Elfring]

>> 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

[PATCH 2/2] powerpc-ps3: Improve a size determination in two functions

[SF Markus Elfring]

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

[PATCH 1/2] powerpc-ps3: Delete an error message for a failed memory allocation in update_flash_db()

[SF Markus Elfring]

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 0/2] PowerPC-PS3: Adjustments for three function implementations

[SF Markus Elfring]

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

Re: char/tpm: Improve a size determination in nine functions

[SF Markus Elfring]

>>  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

Re: [PATCH 0/4] char-TPM: Adjustments for ten function implementations

[SF Markus Elfring]

>>> 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

[PATCH 3/3] powernv/pci: Improve a size determination in pnv_pci_init_ioda_phb()

[SF Markus Elfring]

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

[PATCH 2/3] powernv/pci: Use common code in pnv_ioda_pick_m64_pe()

[SF Markus Elfring]

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 1/3] powernv/pci: Delete an error message for a failed memory allocation in pnv_ioda_pick_m64_pe()

[SF Markus Elfring]

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 0/3] PowerNV-PCI: Adjustments for two function implementations

[SF Markus Elfring]

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 3/3] powerpc-opal: Fix a typo in a comment line of two file headers

[SF Markus Elfring]

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 2/3] powerpc-opal: Improve 12 size determinations

[SF Markus Elfring]

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(_header, (void *)buffer, sizeof(struct image_header_t));
+   memcpy(_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(_node->hmi_evt, hmi_evt, sizeof(struct OpalHMIEvent));
+   memcpy(_node->hmi_evt, hmi_evt, sizeof(*hmi_evt));
 
spin_lock_irqsave(_hmi_evt_lock, flags);
list_add(_node->list, _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(_node->msg, msg, sizeof(struct opal_msg));
+   memcpy(_node->msg, msg, sizeof(msg_node->msg));
 
spin_lock_irqsave(_mem_err_lock, flags);
list_add(_node->list, _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 = 

[PATCH 1/3] powerpc-opal: Delete ten error messages for a failed memory allocation

[SF Markus Elfring]

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(_node->hmi_evt, hmi_evt, sizeof(struct OpalHMIEvent));
 
spin_lock_irqsave(_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(_node->msg, msg, sizeof(struct opal_msg));
 
spin_lock_irqsave(_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, 

[PATCH 0/3] PowerPC-OPAL: Adjustments for some function implementations

[SF Markus Elfring]

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

Re: [PATCH 0/4] char-TPM: Adjustments for ten function implementations

[SF Markus Elfring]

> 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

Re: [PATCH 0/4] char-TPM: Adjustments for ten function implementations

[SF Markus Elfring]

>> 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

[SF Markus Elfring]

> 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

[PATCH 4/4] char/tpm: Less checks in tpm_ibmvtpm_probe() after error detection

[SF Markus Elfring]

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

[PATCH 3/4] char/tpm: Improve a size determination in nine functions

[SF Markus Elfring]

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(>dev, sizeof(struct st33zp24_i2c_phy),
-  GFP_KERNEL);
+   phy = devm_kzalloc(>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, sizeof(struct st33zp24_spi_phy),
-  GFP_KERNEL);
+   phy = devm_kzalloc(>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, 

[PATCH 2/4] char/tpm: Delete an error message for a failed memory allocation in tpm_ibmvtpm_probe()

[SF Markus Elfring]

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 1/4] char/tpm: Delete an error message for a failed memory allocation in tpm_ascii_bios_measurements_show()

[SF Markus Elfring]

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 0/4] char-TPM: Adjustments for ten function implementations

[SF Markus Elfring]

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 2/2] powermac: Improve a size determination in five functions

[SF Markus Elfring]

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(>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 1/2] powermac: Delete an error message for a failed memory allocation in kw_i2c_host_init()

[SF Markus Elfring]

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 0/2] Power Mac: Adjustments for five function implementations

[SF Markus Elfring]

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 2/2] powerpc/platforms/cell: Improve a size determination in three functions

[SF Markus Elfring]

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 1/2] powerpc/platforms/cell: Delete an error message for a failed memory allocation in three functions

[SF Markus Elfring]

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, )) {
pr_err("SPIDERPCI-IOWA:Can't get resource.\n");
-- 
2.14.2

[PATCH 0/2] PowerPC-Cell platform: Adjustments for four function implementations

[SF Markus Elfring]

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] powerpc/perf/hv-24x7: Delete an error message for a failed memory allocation in create_events_from_catalog()

[SF Markus Elfring]

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 2/2] powerpc/oprofile/cell: Improve a size determination in two functions

[SF Markus Elfring]

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 1/2] powerpc/oprofile/cell: Delete an error message for a failed memory allocation in three functions

[SF Markus Elfring]

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 0/2] PowerPC-Cell OProfile: Adjustments for three function implementations

[SF Markus Elfring]

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] KVM: PPC: Book3S HV: Delete an error message for a failed memory allocation in kvmppc_allocate_hpt()

[SF Markus Elfring]

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 v2] axonram: Delete an unnecessary variable initialisation in axon_ram_probe()

[SF Markus Elfring]

From: Markus Elfring 
Date: Tue, 5 Sep 2017 18:47:02 +0200

The local variable "rc" will eventually be set only to an error code.
Thus omit the explicit initialisation at the beginning.

Signed-off-by: Markus Elfring 
---

v2:
Three update steps were integrated for this software module on 2017-09-01.
Thus improve another source code place.

 arch/powerpc/sysdev/axonram.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/sysdev/axonram.c b/arch/powerpc/sysdev/axonram.c
index c60e84e4558d..1b307c80b401 100644
--- a/arch/powerpc/sysdev/axonram.c
+++ b/arch/powerpc/sysdev/axonram.c
@@ -184,7 +184,7 @@ static int axon_ram_probe(struct platform_device *device)
static int axon_ram_bank_id = -1;
struct axon_ram_bank *bank;
struct resource resource;
-   int rc = 0;
+   int rc;
 
axon_ram_bank_id++;
 
-- 
2.14.1

Re: powerpc/eeh: Delete an error message for a failed memory allocation in two functions

[SF Markus Elfring]

> Applied to powerpc next, thanks.
> 
> https://git.kernel.org/powerpc/c/6ab41161b44a3b4d504ac29c9dd997

Thanks that you picked another update suggestion up.

* It might matter to mention that only one patch hunk was accepted in this case.

* Can it be that the adjusted commit subject does contain a typo at the moment?
  Would the wording “powerpc/eeh: Delete a message for an "out of memory error"
  (at init time)” be more appropriate there?

Regards,
Markus

Re: axonram: Delete an error message for a failed memory allocation in axon_ram_probe()

[SF Markus Elfring]

> Series applied to powerpc next, thanks.

Thanks for another positive feedback.

But I wonder how you can refer to the “series” when the forth update step
“Delete an unnecessary variable initialisation” contained a broken suggestion.


> https://git.kernel.org/powerpc/c/a1bddf3991573f3dff2762bfca5e11

This link seems to refer (only?) to the update step “Improve a size 
determination”.
How is the status for the other two change possibilities of this software 
module?

Regards,
Markus

[PATCH] leds-PowerNV: Delete an error message for a failed memory allocation in powernv_led_create()

[SF Markus Elfring]

From: Markus Elfring 
Date: Sun, 27 Aug 2017 22:00:22 +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/leds/leds-powernv.c | 6 +-
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/leds/leds-powernv.c b/drivers/leds/leds-powernv.c
index b2a98c7b521b..b1adbd70ce2e 100644
--- a/drivers/leds/leds-powernv.c
+++ b/drivers/leds/leds-powernv.c
@@ -224,12 +224,8 @@ static int powernv_led_create(struct device *dev,
powernv_led->cdev.name = devm_kasprintf(dev, GFP_KERNEL, "%s:%s",
powernv_led->loc_code,
led_type_desc);
-   if (!powernv_led->cdev.name) {
-   dev_err(dev,
-   "%s: Memory allocation failed for classdev name\n",
-   __func__);
+   if (!powernv_led->cdev.name)
return -ENOMEM;
-   }
 
powernv_led->cdev.brightness_set_blocking = powernv_brightness_set;
powernv_led->cdev.brightness_get = powernv_brightness_get;
-- 
2.14.1

[PATCH] ASoC: Freescale: Delete an error message for a failed memory allocation in three functions

[SF Markus Elfring]

>From 885ccd6c63291dcd4854a0cbaab5188cdc3db805 Mon Sep 17 00:00:00 2001
From: Markus Elfring 
Date: Fri, 11 Aug 2017 15:10:43 +0200
Subject: [PATCH] ASoC: Freescale: Delete an error message for a failed memory 
allocation in three functions

Omit an extra message for a memory allocation failure in these functions.

This issue was detected by using the Coccinelle software.

Link: 
http://events.linuxfoundation.org/sites/events/files/slides/LCJ16-Refactor_Strings-WSang_0.pdf
Signed-off-by: Markus Elfring 
---
 sound/soc/fsl/fsl_asrc_dma.c | 4 +---
 sound/soc/fsl/fsl_dma.c  | 1 -
 sound/soc/fsl/fsl_ssi.c  | 4 +---
 3 files changed, 2 insertions(+), 7 deletions(-)

diff --git a/sound/soc/fsl/fsl_asrc_dma.c b/sound/soc/fsl/fsl_asrc_dma.c
index 282d841840b1..2baf19608bd0 100644
--- a/sound/soc/fsl/fsl_asrc_dma.c
+++ b/sound/soc/fsl/fsl_asrc_dma.c
@@ -282,7 +282,5 @@ static int fsl_asrc_dma_startup(struct snd_pcm_substream 
*substream)
-   if (!pair) {
-   dev_err(dev, "failed to allocate pair\n");
+   if (!pair)
return -ENOMEM;
-   }
 
pair->asrc_priv = asrc_priv;
 
diff --git a/sound/soc/fsl/fsl_dma.c b/sound/soc/fsl/fsl_dma.c
index ccadefceeff2..0ce172f86d6c 100644
--- a/sound/soc/fsl/fsl_dma.c
+++ b/sound/soc/fsl/fsl_dma.c
@@ -907,5 +907,4 @@ static int fsl_soc_dma_probe(struct platform_device *pdev)
if (!dma) {
-   dev_err(>dev, "could not allocate dma object\n");
of_node_put(ssi_np);
return -ENOMEM;
}
diff --git a/sound/soc/fsl/fsl_ssi.c b/sound/soc/fsl/fsl_ssi.c
index 173cb8496641..64598d1183f8 100644
--- a/sound/soc/fsl/fsl_ssi.c
+++ b/sound/soc/fsl/fsl_ssi.c
@@ -1435,7 +1435,5 @@ static int fsl_ssi_probe(struct platform_device *pdev)
-   if (!ssi_private) {
-   dev_err(>dev, "could not allocate DAI object\n");
+   if (!ssi_private)
return -ENOMEM;
-   }
 
ssi_private->soc = of_id->data;
ssi_private->dev = >dev;
-- 
2.14.0

Re: block/ps3vram: Check return of ps3vram_cache_init

[SF Markus Elfring]

> @@ -741,7 +741,11 @@ static int ps3vram_probe(struct ps3_system_bus_device 
> *dev)
>   goto out_unmap_reports;
>   }
>  
> - ps3vram_cache_init(dev);
> + error = ps3vram_cache_init(dev);
> + if (error < 0) {
> + goto out_unmap_reports;
> + }
> +
>   ps3vram_proc_init(dev);
>  
>   queue = blk_alloc_queue(GFP_KERNEL);

I pointed a few development concerns out.
https://patchwork.ozlabs.org/patch/798575/

Will the commit subject “block/ps3vram: Check return value of 
ps3vram_cache_init()”
be more appropriate?

Regards,
Markus

Re: block/ps3vram: Delete an error message for a failed memory allocation in ps3vram_cache_init()

[SF Markus Elfring]

>> https://patchwork.ozlabs.org/patch/798575/
> 
> I submitted your patch

Thanks for your constructive feedback.
https://patchwork.ozlabs.org/patch/798850/


> and a fix to ps3vram_probe() with the other patches in my queue.

I find it nice that you picked this change opportunity up after
a bit of discussion (before an other developer would eventually
have tackled it also).

“Check return of ps3vram_cache_init”
https://patchwork.ozlabs.org/patch/798853/

1. Unfortunately, I find that this specific update suggestion does not fit
   to the Linux coding style convention.

   “…
   Do not unnecessarily use braces where a single statement will do.
   …”

2. How do you think about to use the check “if (error)” instead?

3. Will an additional commit description be useful?

Regards,
Markus

Re: block/ps3vram: Delete an error message for a failed memory allocation in ps3vram_cache_init()

[SF Markus Elfring]

>>> I didn't consider one would be triggered by the kzalloc failure.
>>
>> Do you reconsider any special system settings for further
>> software evolution then?
> 
> Sorry, I don't quite understand your question.

Do you try to configure the Linux error reporting to any special needs?


> I think your original patch is OK,

How does this feedback fit to the initial response “Not Applicable”?
https://patchwork.ozlabs.org/patch/798575/


> and I would appreciate if you added a check for failure of 
> ps3vram_cache_init()
> in ps3vram_probe().

I unsure if this adjustment will need more software updates.


> If you decide not to add that check I'll create a patch for it later.

I am curious on who will pick this update candidate up as the next improvement.
Have you got any preferences for the exception handling there?

Regards,
Markus

Re: block/ps3vram: Delete an error message for a failed memory allocation in ps3vram_cache_init()

[SF Markus Elfring]

>> Do you find the default allocation failure report insufficient?
> 
> The default is OK.

Thanks for this information.


> I didn't consider one would be triggered by the kzalloc failure.

Do you reconsider any special system settings for further
software evolution then?

Regards,
Markus

Re: block/ps3vram: Delete an error message for a failed memory allocation in ps3vram_cache_init()

[SF Markus Elfring]

>> Omit an extra message for a memory allocation failure in this function.
>>
>> This issue was detected by using the Coccinelle software.
> 
> NACK
> 
> When a user asks me for help I would certainly like to get 
> 'Could not allocate cache tags' as apposed to nothing,

Do you find the default allocation failure report insufficient?


> since the return value of ps3vram_cache_init() is not checked.

Are there any more update candidates to consider for better exception handling?

Regards,
Markus

[PATCH] block/ps3vram: Delete an error message for a failed memory allocation in ps3vram_cache_init()

[SF Markus Elfring]

From: Markus Elfring 
Date: Mon, 7 Aug 2017 12:37:01 +0200

Omit an extra message for a memory allocation failure in this function.

This issue was detected by using the Coccinelle software.

Link: 
http://events.linuxfoundation.org/sites/events/files/slides/LCJ16-Refactor_Strings-WSang_0.pdf
Signed-off-by: Markus Elfring 
---
 drivers/block/ps3vram.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/block/ps3vram.c b/drivers/block/ps3vram.c
index e0e81cacd781..ba97d037279e 100644
--- a/drivers/block/ps3vram.c
+++ b/drivers/block/ps3vram.c
@@ -409,10 +409,8 @@ static int ps3vram_cache_init(struct ps3_system_bus_device 
*dev)
priv->cache.page_size = CACHE_PAGE_SIZE;
priv->cache.tags = kzalloc(sizeof(struct ps3vram_tag) *
   CACHE_PAGE_COUNT, GFP_KERNEL);
-   if (priv->cache.tags == NULL) {
-   dev_err(>core, "Could not allocate cache tags\n");
+   if (!priv->cache.tags)
return -ENOMEM;
-   }
 
dev_info(>core, "Created ram cache: %d entries, %d KiB each\n",
CACHE_PAGE_COUNT, CACHE_PAGE_SIZE / 1024);
-- 
2.13.4

[PATCH] powerpc/eeh: Delete an error message for a failed memory allocation in two functions

[SF Markus Elfring]

From: Markus Elfring 
Date: Fri, 4 Aug 2017 16:37:56 +0200

Omit an extra message for a memory allocation failure in these functions.

This issue was detected by using the Coccinelle software.

Link: 
http://events.linuxfoundation.org/sites/events/files/slides/LCJ16-Refactor_Strings-WSang_0.pdf
Signed-off-by: Markus Elfring 
---
 arch/powerpc/kernel/eeh_dev.c   | 5 +
 arch/powerpc/kernel/eeh_event.c | 5 ++---
 2 files changed, 3 insertions(+), 7 deletions(-)

diff --git a/arch/powerpc/kernel/eeh_dev.c b/arch/powerpc/kernel/eeh_dev.c
index d6b2ca70d14d..2742ecbadf18 100644
--- a/arch/powerpc/kernel/eeh_dev.c
+++ b/arch/powerpc/kernel/eeh_dev.c
@@ -55,11 +55,8 @@ struct eeh_dev *eeh_dev_init(struct pci_dn *pdn)
 
/* Allocate EEH device */
edev = kzalloc(sizeof(*edev), GFP_KERNEL);
-   if (!edev) {
-   pr_warn("%s: out of memory\n",
-   __func__);
+   if (!edev)
return NULL;
-   }
 
/* Associate EEH device with OF node */
pdn->edev = edev;
diff --git a/arch/powerpc/kernel/eeh_event.c b/arch/powerpc/kernel/eeh_event.c
index accbf8b5fd46..c62bd90e038f 100644
--- a/arch/powerpc/kernel/eeh_event.c
+++ b/arch/powerpc/kernel/eeh_event.c
@@ -132,10 +132,9 @@ int eeh_send_failure_event(struct eeh_pe *pe)
struct eeh_event *event;
 
event = kzalloc(sizeof(*event), GFP_ATOMIC);
-   if (!event) {
-   pr_err("EEH: out of memory, event not handled\n");
+   if (!event)
return -ENOMEM;
-   }
+
event->pe = pe;
 
/* We may or may not be called in an interrupt context */
-- 
2.13.4