Hello,
I tried another script for the semantic patch language out.
This source code analysis approach points out that the implementation
of the function “_samsung_clk_register_pll” contains also a call
of the function “kmemdup”.
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/clk/samsung/clk-pll.c?id=1c0cc5f1ae5ee5a6913704c0d75a6e99604ee30a#n1275
https://elixir.bootlin.com/linux/v5.4-rc2/source/drivers/clk/samsung/clk-pll.c#L1275
* Do you find the usage of the format string “%s: could not allocate
rate table for %s\n” still appropriate at this place?
* Is there a need to adjust the error handling here?
Regards,
Markus
Hi Markus,
2019年10月12日(土) 23:17 Markus Elfring <[email protected]>:
>
> Hello,
>
> I tried another script for the semantic patch language out.
> This source code analysis approach points out that the implementation
> of the function “_samsung_clk_register_pll” contains also a call
> of the function “kmemdup”.
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/clk/samsung/clk-pll.c?id=1c0cc5f1ae5ee5a6913704c0d75a6e99604ee30a#n1275
> https://elixir.bootlin.com/linux/v5.4-rc2/source/drivers/clk/samsung/clk-pll.c#L1275
Thanks for the report.
>
> * Do you find the usage of the format string “%s: could not allocate
> rate table for %s\n” still appropriate at this place?
Yes, AFAICT there is nothing wrong with that format string.
>
> * Is there a need to adjust the error handling here?
No, there isn't much that can be done if we fail the allocation at
such an early stage.
That said, there is no need to print any warnings or error messages on
allocation failure, so technically they could be removed. It doesn't
really give us anything in case of existing code, though, and only
makes a potential for merge conflicts, so I'd just leave it alone.
Best regards,
Tomasz
> That said, there is no need to print any warnings or error messages on
> allocation failure, so technically they could be removed.
Do you find information sufficient from the Linux allocation failure report?
Regards,
Markus
> That said, there is no need to print any warnings or error messages on
> allocation failure, so technically they could be removed.
Do you find information sufficient from the Linux allocation failure report?
Regards,
Markus
>> * Is there a need to adjust the error handling here?
>
> No, there isn't much that can be done if we fail the allocation at
> such an early stage.
Can it matter to perform the setting “pll->rate_count” only according
to a null pointer check for the variable “pll->rate_table”
because of the function call “kmemdup”?
Regards,
Markus
2019年10月16日(水) 19:55 Markus Elfring <[email protected]>:
>
> >> * Is there a need to adjust the error handling here?
> >
> > No, there isn't much that can be done if we fail the allocation at
> > such an early stage.
>
> Can it matter to perform the setting “pll->rate_count” only according
> to a null pointer check for the variable “pll->rate_table”
> because of the function call “kmemdup”?
It would be a good practice indeed, but looking from the code,
pll->rate_table is checked elsewhere, not pll->rate_count.
Best regards,
Tomasz
2019年10月16日(水) 2:55 Markus Elfring <[email protected]>:
>
> > That said, there is no need to print any warnings or error messages on
> > allocation failure, so technically they could be removed.
>
> Do you find information sufficient from the Linux allocation failure report?
A backtrace should be enough for this kind of a failure that shouldn't
normally happen and if happens, then the rest of the system must be in
a state already about to fail anyway.
Best regards,
Tomasz
> A backtrace should be enough for this kind of a failure that shouldn't
> normally happen and if happens, then the rest of the system must be in
> a state already about to fail anyway.
Does this feedback mean that you would like to omit two extra
error messages from this function implementation?
Regards,
Markus