Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1483601 > unrolled thread

[PATCH 0/4] clk/Renesas-MSTP: Fine-tuning for two function implementations

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH 0/4] clk/Renesas-MSTP: Fine-tuning for two function  implementations SF Markus Elfring <elfring@users.sourceforge.net> - 2016-09-14 22:00 +0200
    [PATCH 4/4] clk/Renesas-MSTP: Rename jump labels in  cpg_mstp_attach_dev() SF Markus Elfring <elfring@users.sourceforge.net> - 2016-09-14 22:10 +0200
      Re: [PATCH 4/4] clk/Renesas-MSTP: Rename jump labels in cpg_mstp_attach_dev() Geert Uytterhoeven <geert@linux-m68k.org> - 2016-09-15 21:20 +0200
        Re: clk/Renesas-MSTP: Rename jump labels in cpg_mstp_attach_dev() SF Markus Elfring <elfring@users.sourceforge.net> - 2016-09-16 08:10 +0200
    [PATCH 1/4] clk/Renesas-MSTP: Use kmalloc_array() in  cpg_mstp_clocks_init() SF Markus Elfring <elfring@users.sourceforge.net> - 2016-09-14 22:10 +0200
      Re: [PATCH 1/4] clk/Renesas-MSTP: Use kmalloc_array() in cpg_mstp_clocks_init() Geert Uytterhoeven <geert@linux-m68k.org> - 2016-09-15 21:20 +0200
      Re: [PATCH 1/4] clk/Renesas-MSTP: Use kmalloc_array() in  cpg_mstp_clocks_init() Stephen Boyd <sboyd@codeaurora.org> - 2016-09-17 01:20 +0200
    [PATCH 3/4] clk/Renesas-MSTP: Less function calls in  cpg_mstp_clocks_init() after error detection SF Markus Elfring <elfring@users.sourceforge.net> - 2016-09-14 22:10 +0200
      Re: [PATCH 3/4] clk/Renesas-MSTP: Less function calls in  cpg_mstp_clocks_init() after error detection Geert Uytterhoeven <geert@linux-m68k.org> - 2016-09-15 21:20 +0200
        Re: [PATCH 3/4] clk/Renesas-MSTP: Less function calls in  cpg_mstp_clocks_init() after error detection Geert Uytterhoeven <geert@linux-m68k.org> - 2016-09-15 22:50 +0200
          Re: clk/Renesas-MSTP: Less function calls in cpg_mstp_clocks_init()  after error detection SF Markus Elfring <elfring@users.sourceforge.net> - 2016-09-16 07:40 +0200
        Re: [PATCH 3/4] clk/Renesas-MSTP: Less function calls in  cpg_mstp_clocks_init() after error detection SF Markus Elfring <elfring@users.sourceforge.net> - 2016-09-15 22:50 +0200
    [PATCH 2/4] clk/Renesas-MSTP: Delete an error message for a failed  memory allocation SF Markus Elfring <elfring@users.sourceforge.net> - 2016-09-14 22:10 +0200
      Re: [PATCH 2/4] clk/Renesas-MSTP: Delete an error message for a  failed memory allocation Geert Uytterhoeven <geert@linux-m68k.org> - 2016-09-15 21:10 +0200
        Re: [PATCH 2/4] clk/Renesas-MSTP: Delete an error message for a failed memory allocation Laurent Pinchart <laurent.pinchart@ideasonboard.com> - 2016-09-15 21:20 +0200
          Re: [PATCH 2/4] clk/Renesas-MSTP: Delete an error message for a  failed memory allocation Wolfram Sang <wsa@the-dreams.de> - 2016-09-15 21:50 +0200
          Re: clk/Renesas-MSTP: Delete an error message for a failed memory  allocation SF Markus Elfring <elfring@users.sourceforge.net> - 2016-09-15 22:20 +0200
            Re: clk/Renesas-MSTP: Delete an error message for a failed memory allocation Laurent Pinchart <laurent.pinchart@ideasonboard.com> - 2016-09-16 01:00 +0200
              Re: clk/Renesas-MSTP: Delete an error message for a failed memory  allocation SF Markus Elfring <elfring@users.sourceforge.net> - 2016-09-16 07:30 +0200

#1483601 — [PATCH 0/4] clk/Renesas-MSTP: Fine-tuning for two function implementations

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 14 Sep 2016 21:48:48 +0200

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

Markus Elfring (4):
  Use kmalloc_array() in cpg_mstp_clocks_init()
  Delete an error message for a failed memory allocation
  Less function calls in cpg_mstp_clocks_init() after error detection
  Rename jump labels in cpg_mstp_attach_dev()

 drivers/clk/renesas/clk-mstp.c | 25 ++++++++++++-------------
 1 file changed, 12 insertions(+), 13 deletions(-)

-- 
2.10.0

[toc] | [next] | [standalone]

#1483609 — [PATCH 4/4] clk/Renesas-MSTP: Rename jump labels in cpg_mstp_attach_dev()

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 14 Sep 2016 21:41:50 +0200

Adjust jump labels according to the current Linux coding style convention.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/clk/renesas/clk-mstp.c | 16 +++++++---------
 1 file changed, 7 insertions(+), 9 deletions(-)

diff --git a/drivers/clk/renesas/clk-mstp.c b/drivers/clk/renesas/clk-mstp.c
index 6c82e0e..2f90718 100644
--- a/drivers/clk/renesas/clk-mstp.c
+++ b/drivers/clk/renesas/clk-mstp.c
@@ -256,19 +256,18 @@ int cpg_mstp_attach_dev(struct generic_pm_domain *unused, struct device *dev)
 					   &clkspec)) {
 		if (of_device_is_compatible(clkspec.np,
 					    "renesas,cpg-mstp-clocks"))
-			goto found;
+			goto get_clk;
 
 		/* BSC on r8a73a4/sh73a0 uses zb_clk instead of an mstp clock */
 		if (!strcmp(clkspec.np->name, "zb_clk"))
-			goto found;
+			goto get_clk;
 
 		of_node_put(clkspec.np);
 		i++;
 	}
 
 	return 0;
-
-found:
+ get_clk:
 	clk = of_clk_get_from_provider(&clkspec);
 	of_node_put(clkspec.np);
 
@@ -278,20 +277,19 @@ found:
 	error = pm_clk_create(dev);
 	if (error) {
 		dev_err(dev, "pm_clk_create failed %d\n", error);
-		goto fail_put;
+		goto put_clk;
 	}
 
 	error = pm_clk_add_clk(dev, clk);
 	if (error) {
 		dev_err(dev, "pm_clk_add_clk %pC failed %d\n", clk, error);
-		goto fail_destroy;
+		goto destroy_clk;
 	}
 
 	return 0;
-
-fail_destroy:
+ destroy_clk:
 	pm_clk_destroy(dev);
-fail_put:
+ put_clk:
 	clk_put(clk);
 	return error;
 }
-- 
2.10.0

[toc] | [prev] | [next] | [standalone]

#1484488 — Re: [PATCH 4/4] clk/Renesas-MSTP: Rename jump labels in cpg_mstp_attach_dev()

Hi Markus,

On Wed, Sep 14, 2016 at 10:04 PM, SF Markus Elfring
<elfring@users.sourceforge.net> wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Wed, 14 Sep 2016 21:41:50 +0200
>
> Adjust jump labels according to the current Linux coding style convention.
>
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
>  drivers/clk/renesas/clk-mstp.c | 16 +++++++---------
>  1 file changed, 7 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/clk/renesas/clk-mstp.c b/drivers/clk/renesas/clk-mstp.c
> index 6c82e0e..2f90718 100644
> --- a/drivers/clk/renesas/clk-mstp.c
> +++ b/drivers/clk/renesas/clk-mstp.c
> @@ -256,19 +256,18 @@ int cpg_mstp_attach_dev(struct generic_pm_domain *unused, struct device *dev)
>                                            &clkspec)) {
>                 if (of_device_is_compatible(clkspec.np,
>                                             "renesas,cpg-mstp-clocks"))
> -                       goto found;
> +                       goto get_clk;
>
>                 /* BSC on r8a73a4/sh73a0 uses zb_clk instead of an mstp clock */
>                 if (!strcmp(clkspec.np->name, "zb_clk"))
> -                       goto found;
> +                       goto get_clk;
>
>                 of_node_put(clkspec.np);
>                 i++;
>         }
>
>         return 0;
> -
> -found:
> + get_clk:

"Choose label names which say what the goto does or why the goto exists."

I prefer the "why" over the "what".

And the "indent labels with a single space" rule will be removed soon, as
there's no longer a technical reason for it after
https://lkml.org/lkml/2016/9/7/316

>         clk = of_clk_get_from_provider(&clkspec);
>         of_node_put(clkspec.np);
>
> @@ -278,20 +277,19 @@ found:
>         error = pm_clk_create(dev);
>         if (error) {
>                 dev_err(dev, "pm_clk_create failed %d\n", error);
> -               goto fail_put;
> +               goto put_clk;
>         }
>
>         error = pm_clk_add_clk(dev, clk);
>         if (error) {
>                 dev_err(dev, "pm_clk_add_clk %pC failed %d\n", clk, error);
> -               goto fail_destroy;
> +               goto destroy_clk;
>         }
>
>         return 0;
> -
> -fail_destroy:
> + destroy_clk:
>         pm_clk_destroy(dev);
> -fail_put:
> + put_clk:

Same here.

>         clk_put(clk);
>         return error;
>  }

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

[toc] | [prev] | [next] | [standalone]

#1484695 — Re: clk/Renesas-MSTP: Rename jump labels in cpg_mstp_attach_dev()

> "Choose label names which say what the goto does or why the goto exists."
> 
> I prefer the "why" over the "what".

Does your opinion indicate also that you would appreciate another adjustment
around the quoted sentence from "Chapter 7: Centralized exiting of functions"
of the document "CodingStyle"?

Would you like to achieve that the potential for confusion will be reduced
a bit more there?


> And the "indent labels with a single space" rule will be removed soon,

I am unsure on how this "story" will evolve further.

Will the indentation rules become any more precise for Linux source code?


> as there's no longer a technical reason for it after
> https://lkml.org/lkml/2016/9/7/316

The software update "Set git diff driver for C source code files" is also
interesting for current versions.

Will language-specific rules which are supported by recent Git software
influence any capabilities for the command "diff --show-c-function"?

Regards,
Markus

[toc] | [prev] | [next] | [standalone]

#1483612 — [PATCH 1/4] clk/Renesas-MSTP: Use kmalloc_array() in cpg_mstp_clocks_init()

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 14 Sep 2016 21:10:47 +0200

A multiplication for the size determination of a memory allocation
indicated that an array data structure should be processed.
Thus use the corresponding function "kmalloc_array".

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/clk/renesas/clk-mstp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/renesas/clk-mstp.c b/drivers/clk/renesas/clk-mstp.c
index 5093a25..9375777 100644
--- a/drivers/clk/renesas/clk-mstp.c
+++ b/drivers/clk/renesas/clk-mstp.c
@@ -167,7 +167,7 @@ static void __init cpg_mstp_clocks_init(struct device_node *np)
 	unsigned int i;
 
 	group = kzalloc(sizeof(*group), GFP_KERNEL);
-	clks = kmalloc(MSTP_MAX_CLOCKS * sizeof(*clks), GFP_KERNEL);
+	clks = kmalloc_array(MSTP_MAX_CLOCKS, sizeof(*clks), GFP_KERNEL);
 	if (group == NULL || clks == NULL) {
 		kfree(group);
 		kfree(clks);
-- 
2.10.0

[toc] | [prev] | [next] | [standalone]

#1484477 — Re: [PATCH 1/4] clk/Renesas-MSTP: Use kmalloc_array() in cpg_mstp_clocks_init()

On Wed, Sep 14, 2016 at 10:00 PM, SF Markus Elfring
<elfring@users.sourceforge.net> wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Wed, 14 Sep 2016 21:10:47 +0200
>
> A multiplication for the size determination of a memory allocation
> indicated that an array data structure should be processed.
> Thus use the corresponding function "kmalloc_array".
>
> This issue was detected by using the Coccinelle software.
>
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

[toc] | [prev] | [next] | [standalone]

#1485418 — Re: [PATCH 1/4] clk/Renesas-MSTP: Use kmalloc_array() in cpg_mstp_clocks_init()

On 09/14, SF Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Wed, 14 Sep 2016 21:10:47 +0200
> 
> A multiplication for the size determination of a memory allocation
> indicated that an array data structure should be processed.
> Thus use the corresponding function "kmalloc_array".
> 
> This issue was detected by using the Coccinelle software.
> 
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---

Applied to clk-next

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

[toc] | [prev] | [next] | [standalone]

#1483615 — [PATCH 3/4] clk/Renesas-MSTP: Less function calls in cpg_mstp_clocks_init() after error detection

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 14 Sep 2016 21:30:27 +0200

The kfree() function was called in up to two cases
by the cpg_mstp_clocks_init() function during error handling even if
the passed variable contained a null pointer.

* Split a condition check for memory allocation failures so that
  each pointer from these function calls will be checked immediately.

  See also background information:
  Topic "CWE-754: Improper check for unusual or exceptional conditions"
  Link: https://cwe.mitre.org/data/definitions/754.html

* Return directly after a call of the function "kzalloc" failed
  at the beginning.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/clk/renesas/clk-mstp.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/renesas/clk-mstp.c b/drivers/clk/renesas/clk-mstp.c
index 1fdc44b..6c82e0e 100644
--- a/drivers/clk/renesas/clk-mstp.c
+++ b/drivers/clk/renesas/clk-mstp.c
@@ -167,10 +167,12 @@ static void __init cpg_mstp_clocks_init(struct device_node *np)
 	unsigned int i;
 
 	group = kzalloc(sizeof(*group), GFP_KERNEL);
+	if (!group)
+		return;
+
 	clks = kmalloc_array(MSTP_MAX_CLOCKS, sizeof(*clks), GFP_KERNEL);
-	if (group == NULL || clks == NULL) {
+	if (!clks) {
 		kfree(group);
-		kfree(clks);
 		return;
 	}
 
-- 
2.10.0

[toc] | [prev] | [next] | [standalone]

#1484486 — Re: [PATCH 3/4] clk/Renesas-MSTP: Less function calls in cpg_mstp_clocks_init() after error detection

Hi Markus,

On Wed, Sep 14, 2016 at 10:03 PM, SF Markus Elfring
<elfring@users.sourceforge.net> wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Wed, 14 Sep 2016 21:30:27 +0200
>
> The kfree() function was called in up to two cases
> by the cpg_mstp_clocks_init() function during error handling even if
> the passed variable contained a null pointer.

It's perfectly legal to call kfree() on a NULL pointer.

> * Split a condition check for memory allocation failures so that
>   each pointer from these function calls will be checked immediately.
>
>   See also background information:
>   Topic "CWE-754: Improper check for unusual or exceptional conditions"
>   Link: https://cwe.mitre.org/data/definitions/754.html
>
> * Return directly after a call of the function "kzalloc" failed
>   at the beginning.

Both calls are already close together.

> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
>  drivers/clk/renesas/clk-mstp.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)

In addition, your patch increases the LoC, IMHO without improving the code.

>
> diff --git a/drivers/clk/renesas/clk-mstp.c b/drivers/clk/renesas/clk-mstp.c
> index 1fdc44b..6c82e0e 100644
> --- a/drivers/clk/renesas/clk-mstp.c
> +++ b/drivers/clk/renesas/clk-mstp.c
> @@ -167,10 +167,12 @@ static void __init cpg_mstp_clocks_init(struct device_node *np)
>         unsigned int i;
>
>         group = kzalloc(sizeof(*group), GFP_KERNEL);
> +       if (!group)
> +               return;
> +
>         clks = kmalloc_array(MSTP_MAX_CLOCKS, sizeof(*clks), GFP_KERNEL);
> -       if (group == NULL || clks == NULL) {
> +       if (!clks) {
>                 kfree(group);
> -               kfree(clks);
>                 return;
>         }

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

[toc] | [prev] | [next] | [standalone]

#1484530 — Re: [PATCH 3/4] clk/Renesas-MSTP: Less function calls in cpg_mstp_clocks_init() after error detection

On Thu, Sep 15, 2016 at 10:40 PM, SF Markus Elfring
<elfring@users.sourceforge.net> wrote:
>>> * Split a condition check for memory allocation failures so that
>>>   each pointer from these function calls will be checked immediately.
>>>
>>>   See also background information:
>>>   Topic "CWE-754: Improper check for unusual or exceptional conditions"
>>>   Link: https://cwe.mitre.org/data/definitions/754.html
>>>
>>> * Return directly after a call of the function "kzalloc" failed
>>>   at the beginning.
>>
>> Both calls are already close together.
>
> Can it be that an other software development concern is eventually
> overlooked because of this "neighbourship" (or is categorised with
> a lower priority)?
>
> I suggest to reconsider this design detail if it is really acceptable
> for the safe implementation of such a software module.
>
> * How much will it matter in general that one function call was performed
>   in this use case without checking its return values immediately?
>
> * Should it usually be determined quicker if a required resource like
>   memory could be acquired before trying the next allocation?

Note that if memory allocation fails in this driver, the system won't
boot at all. So even not checking for allocation failures at all could be
acceptable.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

[toc] | [prev] | [next] | [standalone]

#1484673 — Re: clk/Renesas-MSTP: Less function calls in cpg_mstp_clocks_init() after error detection

>> * Should it usually be determined quicker if a required resource like
>>   memory could be acquired before trying the next allocation?
> 
> Note that if memory allocation fails in this driver, the system won't
> boot at all.

Thanks for this information.


> So even not checking for allocation failures at all could be acceptable.

I find this opinion interesting somehow.

I would generally prefer to check return values from various function calls
immediately instead of keeping the discussed source code structure unchanged.

Regards,
Markus

[toc] | [prev] | [next] | [standalone]

#1484534 — Re: [PATCH 3/4] clk/Renesas-MSTP: Less function calls in cpg_mstp_clocks_init() after error detection

> It's perfectly legal to call kfree() on a NULL pointer.

I know this function property well.


>> * Split a condition check for memory allocation failures so that
>>   each pointer from these function calls will be checked immediately.
>>
>>   See also background information:
>>   Topic "CWE-754: Improper check for unusual or exceptional conditions"
>>   Link: https://cwe.mitre.org/data/definitions/754.html
>>
>> * Return directly after a call of the function "kzalloc" failed
>>   at the beginning.
> 
> Both calls are already close together.

Can it be that an other software development concern is eventually
overlooked because of this "neighbourship" (or is categorised with
a lower priority)?

I suggest to reconsider this design detail if it is really acceptable
for the safe implementation of such a software module.

* How much will it matter in general that one function call was performed
  in this use case without checking its return values immediately?

* Should it usually be determined quicker if a required resource like
  memory could be acquired before trying the next allocation?


> In addition, your patch increases the LoC, IMHO without improving the code.

I find this consequence still debatable.


>> diff --git a/drivers/clk/renesas/clk-mstp.c b/drivers/clk/renesas/clk-mstp.c
>> index 1fdc44b..6c82e0e 100644
>> --- a/drivers/clk/renesas/clk-mstp.c
>> +++ b/drivers/clk/renesas/clk-mstp.c
>> @@ -167,10 +167,12 @@ static void __init cpg_mstp_clocks_init(struct device_node *np)
>>         unsigned int i;
>>
>>         group = kzalloc(sizeof(*group), GFP_KERNEL);
>> +       if (!group)
>> +               return;
>> +
>>         clks = kmalloc_array(MSTP_MAX_CLOCKS, sizeof(*clks), GFP_KERNEL);
>> -       if (group == NULL || clks == NULL) {
>> +       if (!clks) {
>>                 kfree(group);
>> -               kfree(clks);
>>                 return;
>>         }

Is this update suggestion worth for another look?

Regards,
Markus

[toc] | [prev] | [next] | [standalone]

#1483616 — [PATCH 2/4] clk/Renesas-MSTP: Delete an error message for a failed memory allocation

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 14 Sep 2016 21:17:18 +0200

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

Link: http://events.linuxfoundation.org/sites/events/files/slides/LCJ16-Refactor_Strings-WSang_0.pdf

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/clk/renesas/clk-mstp.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/clk/renesas/clk-mstp.c b/drivers/clk/renesas/clk-mstp.c
index 9375777..1fdc44b 100644
--- a/drivers/clk/renesas/clk-mstp.c
+++ b/drivers/clk/renesas/clk-mstp.c
@@ -171,7 +171,6 @@ static void __init cpg_mstp_clocks_init(struct device_node *np)
 	if (group == NULL || clks == NULL) {
 		kfree(group);
 		kfree(clks);
-		pr_err("%s: failed to allocate group\n", __func__);
 		return;
 	}
 
-- 
2.10.0

[toc] | [prev] | [next] | [standalone]

#1484474 — Re: [PATCH 2/4] clk/Renesas-MSTP: Delete an error message for a failed memory allocation

On Wed, Sep 14, 2016 at 10:01 PM, SF Markus Elfring
<elfring@users.sourceforge.net> wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Wed, 14 Sep 2016 21:17:18 +0200
>
> Omit an extra message for a memory allocation failure in this function.
>
> Link: http://events.linuxfoundation.org/sites/events/files/slides/LCJ16-Refactor_Strings-WSang_0.pdf
>
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

[toc] | [prev] | [next] | [standalone]

#1484487 — Re: [PATCH 2/4] clk/Renesas-MSTP: Delete an error message for a failed memory allocation

On Thursday 15 Sep 2016 21:07:17 Geert Uytterhoeven wrote:
> On Wed, Sep 14, 2016 at 10:01 PM, SF Markus Elfring wrote:
> > From: Markus Elfring <elfring@users.sourceforge.net>
> > Date: Wed, 14 Sep 2016 21:17:18 +0200
> > 
> > Omit an extra message for a memory allocation failure in this function.
> > 
> > Link:
> > http://events.linuxfoundation.org/sites/events/files/slides/LCJ16-Refacto
> > r_Strings-WSang_0.pdf
> > 
> > Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> 
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

There are two other memory allocation failures printed by drivers in the same 
directory that, you'll get my ack if you extend this patch to remove the three 
messages in one go.

-- 
Regards,

Laurent Pinchart

[toc] | [prev] | [next] | [standalone]

#1484504 — Re: [PATCH 2/4] clk/Renesas-MSTP: Delete an error message for a failed memory allocation

[Multipart message — attachments visible in raw view] — view raw

On Thu, Sep 15, 2016 at 10:13:17PM +0300, Laurent Pinchart wrote:
> On Thursday 15 Sep 2016 21:07:17 Geert Uytterhoeven wrote:
> > On Wed, Sep 14, 2016 at 10:01 PM, SF Markus Elfring wrote:
> > > From: Markus Elfring <elfring@users.sourceforge.net>
> > > Date: Wed, 14 Sep 2016 21:17:18 +0200
> > > 
> > > Omit an extra message for a memory allocation failure in this function.
> > > 
> > > Link:
> > > http://events.linuxfoundation.org/sites/events/files/slides/LCJ16-Refacto
> > > r_Strings-WSang_0.pdf
> > > 
> > > Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> > 
> > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> 
> There are two other memory allocation failures printed by drivers in the same 
> directory that, you'll get my ack if you extend this patch to remove the three 
> messages in one go.

I agree.

[toc] | [prev] | [next] | [standalone]

#1484519 — Re: clk/Renesas-MSTP: Delete an error message for a failed memory allocation

>>> Omit an extra message for a memory allocation failure in this function.
>>>
>>> Link:
>>> http://events.linuxfoundation.org/sites/events/files/slides/LCJ16-Refacto
>>> r_Strings-WSang_0.pdf
>>>
>>> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
>>
>> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> 
> There are two other memory allocation failures printed by drivers in the same 
> directory that, you'll get my ack

Does this kind of feedback express a general acceptance for the deletion
of similar error messages?


> if you extend this patch to remove the three messages in one go.

Does this wish influence the handling of suggested changes for the source file
"drivers/clk/renesas/clk-mstp.c" anyhow?
https://patchwork.kernel.org/patch/9332363/

Regards,
Markus

[toc] | [prev] | [next] | [standalone]

#1484607 — Re: clk/Renesas-MSTP: Delete an error message for a failed memory allocation

On Thursday 15 Sep 2016 22:17:41 SF Markus Elfring wrote:
> >>> Omit an extra message for a memory allocation failure in this function.
> >>> 
> >>> Link:
> >>> http://events.linuxfoundation.org/sites/events/files/slides/LCJ16-Refact
> >>> or_Strings-WSang_0.pdf
> >>> 
> >>> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> >> 
> >> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> > 
> > There are two other memory allocation failures printed by drivers in the
> > same directory that, you'll get my ack
> 
> Does this kind of feedback express a general acceptance for the deletion
> of similar error messages?

No, it's my opinion only.

> > if you extend this patch to remove the three messages in one go.
> 
> Does this wish influence the handling of suggested changes for the source
> file "drivers/clk/renesas/clk-mstp.c" anyhow?
> https://patchwork.kernel.org/patch/9332363/

Yes, please submit a new version of this patch that removes all the memory 
allocation error messages from drivers/clk/renesas/ in one go.

-- 
Regards,

Laurent Pinchart

[toc] | [prev] | [next] | [standalone]

#1484667 — Re: clk/Renesas-MSTP: Delete an error message for a failed memory allocation

>> Does this wish influence the handling of suggested changes for the source
>> file "drivers/clk/renesas/clk-mstp.c" anyhow?
>> https://patchwork.kernel.org/patch/9332363/
> 
> Yes, please submit a new version of this patch that removes all the memory 
> allocation error messages from drivers/clk/renesas/ in one go.

I find this kind of feedback a bit surprising and it seems to be promising
to increase software development in this design direction.
Have you got any corresponding scripts for the semantic patch language
prepared to make such a source code adjustment safer (and a bit more convenient
with the help of the Coccinelle software)?

I am also curious on how the change acceptance will evolve for the other
three update steps from this patch series when this one indicates further
software update opportunities already.

Regards,
Markus

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web