Re: [PATCH 1/3] driver core: generalize driver_override in struct device

["Danilo Krummrich" <dakr@kernel.org>]

On Mon Mar 2, 2026 at 11:00 AM CET, Geert Uytterhoeven wrote:
>> --- a/drivers/base/dd.c
>> +++ b/drivers/base/dd.c
>> @@ -381,6 +381,66 @@ static void __exit deferred_probe_exit(void)
>>  }
>>  __exitcall(deferred_probe_exit);
>>
>> +int __device_set_driver_override(struct device *dev, const char *s, size_t len)
>> +{
>> +       const char *new, *old;
>> +       char *cp;
>> +
>> +       if (!s)
>> +               return -EINVAL;
>> +
>> +       /*
>> +        * The stored value will be used in sysfs show callback (sysfs_emit()),
>> +        * which has a length limit of PAGE_SIZE and adds a trailing newline.
>> +        * Thus we can store one character less to avoid truncation during sysfs
>> +        * show.
>> +        */
>> +       if (len >= (PAGE_SIZE - 1))
>> +               return -EINVAL;
>> +
>> +       /*
>> +        * Compute the real length of the string in case userspace sends us a
>> +        * bunch of \0 characters like python likes to do.
>> +        */
>> +       len = strlen(s);
>> +
>
> The newline case below is is basically the same case as the empty
> string.  Hence if you would move the newline check here:
>
>     if (len) {
>             cp = strnchr(s, len, '\n');
>             if (cp)
>                     len = cp - s;
>     }
>
> then the "cp != s" check below is no longer needed.
>
>> +       if (!len) {
>> +               /* Empty string passed - clear override */
>> +               spin_lock(&dev->driver_override.lock);
>> +               old = dev->driver_override.name;
>> +               dev->driver_override.name = NULL;
>> +               spin_unlock(&dev->driver_override.lock);
>> +               kfree(old);
>> +
>> +               return 0;
>> +       }
>
> Also, this block can be eliminated completely...
>
>> +
>> +       cp = strnchr(s, len, '\n');
>> +       if (cp)
>> +               len = cp - s;
>> +
>> +       new = kstrndup(s, len, GFP_KERNEL);
>> +       if (!new)
>> +               return -ENOMEM;
>
> ... by pre-initializing new to NULL, and making the allocation of new
> conditional on len being non-zero.
>
>> +
>> +       spin_lock(&dev->driver_override.lock);
>> +       old = dev->driver_override.name;
>> +       if (cp != s) {
>> +               dev->driver_override.name = new;
>> +               spin_unlock(&dev->driver_override.lock);
>> +       } else {
>> +               /* "\n" passed - clear override */
>> +               dev->driver_override.name = NULL;
>> +               spin_unlock(&dev->driver_override.lock);
>> +
>> +               kfree(new);
>> +       }
>> +       kfree(old);
>> +
>> +       return 0;
>> +}
>> +EXPORT_SYMBOL_GPL(__device_set_driver_override);

This is essentially a copy of driver_set_override(). Except for the required
minor changes I intentionally kept it "as is" as it will go through -fixes and
we know it works properly.

Do you mind sending a follow-up patch with your suggested improvements?