On Wed, Dec 18, 2024 at 5:35 AM Petr Mladek <pmladek@xxxxxxxx> wrote: > > On Tue 2024-12-17 12:37:09, 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. > > > > --- a/drivers/of/module.c > > +++ b/drivers/of/module.c > > @@ -8,21 +8,14 @@ > > #include <linux/slab.h> > > #include <linux/string.h> > > > > -ssize_t of_modalias(const struct device_node *np, char *str, ssize_t len) > > +/* Do not use directly, use %pOFm format specifier instead */ > > +size_t of_modalias(const struct device_node *np, char *str, size_t len) > > We should keep ssize_t. My intent was to align of_modalias() with snprintf()... > "end - buf" passed from device_node_string() in vprintf.c might be > negative. The "buf" pointer is used to count the number of characters > which might be written when the buffer is big enough. Isn't Rasmus' suggestion sufficient?: buf += of_modalias(dn, buf, buf < end ? end - buf : 0) Rob
