Hello.
On 2/11/2016 12:14 PM, Petr Kulhavy wrote:
+ if (!data) {
+ ret = -ENOMEM;
+ goto err5;
+ }
+
+ config = devm_kzalloc(&pdev->dev, sizeof(*config), GFP_KERNEL);
Why not just use the static config?!
I would say bad example, again :-)
You're supposed to think a bit, not just copy&paste. ;-)
BTW the config pointer in struct musb_hdrc_platform_data should be const to
make sure the musb core doesn't alter it.
I'll clean up that section.
It is a bad practice to point to a static const structure with a non-constant
pointer.
Because anyone using that pointer can alter it. Imagine 2 instances of the
driver, both pointing to the same data.
One instance changes the data and it affects the other instance. That's why
the allocate-copy strategy is in fact correct
with the way the pointer is defined.
However, the current musb_core uses the pointer read-only so the above
situation practically does not happen, as far I can see.
So either the intention was to use it read-only, then the pointer should be
const. Or it was supposed to write to it and then
the structure should be allocated and copied (which is currently missing e.g.
in the ux500 driver).
No, the driver usually isn't supposed to write to the platform data in
general.
So do you agree that I change the pointer to const ?
Yes, of course.
Regards
Petr
MBR, Sergei
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
