[PATCH] MIPS: Alchemy: Delete an error message for a failed memory allocation in alchemy_pci_probe()

[PATCH] MIPS: Alchemy: Delete an error message for a failed memory allocation in alchemy_pci_probe()

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 24 May 2017 18:32:21 +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 <elfring@users.sourceforge.net>
---
 arch/mips/pci/pci-alchemy.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/arch/mips/pci/pci-alchemy.c b/arch/mips/pci/pci-alchemy.c
index e99ca7702d8a..a58c3290bd4e 100644
--- a/arch/mips/pci/pci-alchemy.c
+++ b/arch/mips/pci/pci-alchemy.c
@@ -377,7 +377,6 @@ static int alchemy_pci_probe(struct platform_device *pdev)
 
 	ctx = kzalloc(sizeof(*ctx), GFP_KERNEL);
 	if (!ctx) {
-		dev_err(&pdev->dev, "no memory for pcictl context\n");
 		ret = -ENOMEM;
 		goto out;
 	}
-- 
2.13.0

Re: [PATCH] MIPS: Alchemy: Delete an error message for a failed memory allocation in alchemy_pci_pro

[Manuel Lauss]

On Wed, May 24, 2017 at 6:42 PM, SF Markus Elfring
<elfring@users.sourceforge.net> wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Wed, 24 May 2017 18:32:21 +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 <elfring@users.sourceforge.net>
> ---
>  arch/mips/pci/pci-alchemy.c | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/arch/mips/pci/pci-alchemy.c b/arch/mips/pci/pci-alchemy.c
> index e99ca7702d8a..a58c3290bd4e 100644
> --- a/arch/mips/pci/pci-alchemy.c
> +++ b/arch/mips/pci/pci-alchemy.c
> @@ -377,7 +377,6 @@ static int alchemy_pci_probe(struct platform_device *pdev)
>
>         ctx = kzalloc(sizeof(*ctx), GFP_KERNEL);
>         if (!ctx) {
> -               dev_err(&pdev->dev, "no memory for pcictl context\n");
>                 ret = -ENOMEM;
>                 goto out;
>         }
> --
> 2.13.0

Why are you removing just this one dev_err()?  What issue are you
trying to address?

Manuel

Re: [PATCH] MIPS: Alchemy: Delete an error message for a failed memory allocation in alchemy_pci_pro

[SF Markus Elfring]

>> +++ b/arch/mips/pci/pci-alchemy.c
>> @@ -377,7 +377,6 @@ static int alchemy_pci_probe(struct platform_device *pdev)
>>
>>         ctx = kzalloc(sizeof(*ctx), GFP_KERNEL);
>>         if (!ctx) {
>> -               dev_err(&pdev->dev, "no memory for pcictl context\n");
>>                 ret = -ENOMEM;
>>                 goto out;
>>         }
>> --
>> 2.13.0
> 
> Why are you removing just this one dev_err()?

How do you think about to achieve a small code reduction also for this software module?


> What issue are you trying to address?

Do you find information from a Linux allocation failure report sufficient
for such a function implementation?

Regards,
Markus

Re: [PATCH] MIPS: Alchemy: Delete an error message for a failed memory allocation in alchemy_pci_pro

[Randy Dunlap]

On 05/24/17 11:15, SF Markus Elfring wrote:
>>> +++ b/arch/mips/pci/pci-alchemy.c
>>> @@ -377,7 +377,6 @@ static int alchemy_pci_probe(struct platform_device *pdev)
>>>
>>>         ctx = kzalloc(sizeof(*ctx), GFP_KERNEL);
>>>         if (!ctx) {
>>> -               dev_err(&pdev->dev, "no memory for pcictl context\n");
>>>                 ret = -ENOMEM;
>>>                 goto out;
>>>         }
>>> --
>>> 2.13.0
>>
>> Why are you removing just this one dev_err()?

Agreed.  Why just this one error message when there are many in this function?

> How do you think about to achieve a small code reduction also for this software module?
> 
> 
>> What issue are you trying to address?
> 
> Do you find information from a Linux allocation failure report sufficient
> for such a function implementation?

Can you answer questions with statements instead of with questions?

thanks.
-- 
~Randy

Re: MIPS: Alchemy: Delete an error message for a failed memory allocation in alchemy_pci_probe()

[SF Markus Elfring]

> Why just this one error message when there are many in this function?

A bigger software update depends on corresponding change acceptance
and consensus for the evolution of affected error handling, doesn't it?

Regards,
Markus

Re: [PATCH] MIPS: Alchemy: Delete an error message for a failed memory allocation in alchemy_pci_pro

[Manuel Lauss]

On Wed, May 24, 2017 at 8:15 PM, SF Markus Elfring
<elfring@users.sourceforge.net> wrote:
>>> +++ b/arch/mips/pci/pci-alchemy.c
>>> @@ -377,7 +377,6 @@ static int alchemy_pci_probe(struct platform_device *pdev)
>>>
>>>         ctx = kzalloc(sizeof(*ctx), GFP_KERNEL);
>>>         if (!ctx) {
>>> -               dev_err(&pdev->dev, "no memory for pcictl context\n");
>>>                 ret = -ENOMEM;
>>>                 goto out;
>>>         }
>>> --
>>> 2.13.0
>>
>> Why are you removing just this one dev_err()?
>
> How do you think about to achieve a small code reduction also for this software module?

Generally speaking, sure.  But why remove just this one?  Is it
because it loosely follows a
pattern that was deemed removable in that slidedeck you linked to?
(the "usb_submit_urb()" part)?


>> What issue are you trying to address?
>
> Do you find information from a Linux allocation failure report sufficient
> for such a function implementation?

Yes, I wrote that code, and in case this driver doesn't load, I'd like
to know precisely where
initialization failed.  I can happily spare a few bytes for that.

Manuel

Re: MIPS: Alchemy: Delete an error message for a failed memory allocation in alchemy_pci_probe()

[SF Markus Elfring]

>> How do you think about to achieve a small code reduction also for this software module?
> 
> Generally speaking, sure.

Thanks for your interest in such a direction.


> But why remove just this one?  Is it because it loosely follows a
> pattern that was deemed removable in that slidedeck you linked to?

I derived another source code search approach from the implementation
of the check “OOM_MESSAGE” in the script “checkpatch.pl” for
the semantic patch language (Coccinelle software).
The involved search patterns are still evolving and the used lists
(or regular expressions) for function names where it might make sense
to reconsider the usage of special logging calls is therefore incomplete.


> (the "usb_submit_urb()" part)?

Would you like to extend the function selection for further considerations?


>> Do you find information from a Linux allocation failure report sufficient
>> for such a function implementation?
> 
> Yes, I wrote that code, and in case this driver doesn't load, I'd like
> to know precisely where initialization failed.
> I can happily spare a few bytes for that.

Does this kind of answer contain a bit of contradiction?

* Why do you seem to insist on another message if information from a Linux
  allocation failure report would be sufficient already also for this
  software module?

* Do you want that it can become easier to map a position in a backtrace
  to a place in your source code?

Regards,
Markus

Re: MIPS: Alchemy: Delete an error message for a failed memory allocation in alchemy_pci_probe()

[Manuel Lauss]

On Thu, May 25, 2017 at 8:54 AM, SF Markus Elfring
<elfring@users.sourceforge.net> wrote:
>>> How do you think about to achieve a small code reduction also for this software module?
>>
>> Generally speaking, sure.
>
> Thanks for your interest in such a direction.
>
>
>> But why remove just this one?  Is it because it loosely follows a
>> pattern that was deemed removable in that slidedeck you linked to?
>
> I derived another source code search approach from the implementation
> of the check “OOM_MESSAGE” in the script “checkpatch.pl” for
> the semantic patch language (Coccinelle software).
> The involved search patterns are still evolving and the used lists
> (or regular expressions) for function names where it might make sense
> to reconsider the usage of special logging calls is therefore incomplete.
>
>
>> (the "usb_submit_urb()" part)?
>
> Would you like to extend the function selection for further considerations?
>
>
>>> Do you find information from a Linux allocation failure report sufficient
>>> for such a function implementation?
>>
>> Yes, I wrote that code, and in case this driver doesn't load, I'd like
>> to know precisely where initialization failed.
>> I can happily spare a few bytes for that.
>
> Does this kind of answer contain a bit of contradiction?
>
> * Why do you seem to insist on another message if information from a Linux
>   allocation failure report would be sufficient already also for this
>   software module?
>
> * Do you want that it can become easier to map a position in a backtrace
>   to a place in your source code?

Does kmalloc() nowadays print a message which invocation (source line) failed?
If so I won't be standing in your way, but if not, you need to come up with
something for convincing than answering questions with more questions.

Manuel

Re: MIPS: Alchemy: Delete an error message for a failed memory allocation in alchemy_pci_probe()

[SF Markus Elfring]

>> * Why do you seem to insist on another message if information from a Linux
>>   allocation failure report would be sufficient already also for this
>>   software module?
>>
>> * Do you want that it can become easier to map a position in a backtrace
>>   to a place in your source code?
> 
> Does kmalloc() nowadays print a message which invocation (source line) failed?
> If so I won't be standing in your way, but if not, you need to come up with
> something for convincing than answering questions with more questions.

I suggest also to improve the corresponding documentation for the affected
programming interfaces in significant ways.
How are the chances for the desired clarification of relevant implementation details?

Regards,
Markus