On Mon, Dec 23, 2024 at 09:58:27PM +0200, Andy Shevchenko wrote: > On Tue, Dec 17, 2024 at 12:37:09PM -0600, Rob Herring (Arm) wrote: > > The callers for of_modalias() generally need the module alias as part of > > some larger string. That results in some error prone manipulation of the > > buffer prepend/append the module alias string. In fact, > > of_device_uevent_modalias() has several issues. First, it's off by one > > too few characters in utilization of the full buffer. Second, the error > > paths leave OF_MODALIAS with a truncated value when in the end nothing > > should be added to the buffer. It is also fragile because it needs > > internal details of struct kobj_uevent_env. add_uevent_var() really > > wants to write the env variable and value in one shot which would need > > either a temporary buffer for value or a format specifier. > > > > Fix these issues by adding a new printf format specifier, "%pOFm". With > > the format specifier in place, simplify all the callers of > > of_modalias(). of_modalias() can also be simplified with vsprintf() > > being the only caller as it avoids the error conditions. > > Shouldn't ACPI case also be considered? Otherwise we might see a deviation and > then completely asynced variants of modalias based on different type of fwnode. I don't understand what you are proposing. How the ACPI code and DT code generate the DT modalias are completely different already. Are you wanting to plumb a fwnode modalias op? We'd probably be better off with a printf op handler and moving much of the code out of lib/vsprintf.c. Also, looks like the ACPI modalias code does kmallocs. Given the locking discussion, wouldn't that be problematic? Rob
