On Sun, Mar 1, 2020 at 3:58 AM Bart Van Assche <bvanassche@xxxxxxx> wrote:
>
> On 2020-02-21 02:47, Jack Wang wrote:
> > +static int dev_search_path_set(const char *val, const struct kernel_param *kp)
> > +{
> > + char *dup;
> > +
> > + if (strlen(val) >= sizeof(dev_search_path))
> > + return -EINVAL;
> > +
> > + dup = kstrdup(val, GFP_KERNEL);
> > +
> > + if (dup[strlen(dup) - 1] == '\n')
> > + dup[strlen(dup) - 1] = '\0';
> > +
> > + strlcpy(dev_search_path, dup, sizeof(dev_search_path));
> > +
> > + kfree(dup);
> > + pr_info("dev_search_path changed to '%s'\n", dev_search_path);
> > +
> > + return 0;
> > +}
>
> It is not necessary in this function to do memory allocation. Something
> like the following (untested) code should be sufficient:
>
> const char *p = strrchr(val, '\n') ? : val + strlen(val);
>
> snprintf(dev_search_path, sizeof(dev_search_path), "%.*s",
> (int)(p - val), val);
>
> How are concurrent attempts to change dev_search_path serialized?
>
Hi Bart,
thanks a lot for your comments. Will try to avoid the allocation. The
module parameter is readonly. It's only set during module init - I
guess we don't need to handle concurrent access?
Best,
Danil.
> Thanks,
>
> Bart.
