Re: [PATCH v9 20/25] block/rnbd: server: main functionality

[Danil Kipnis <danil.kipnis@cloud.ionos.com>]

On Sun, Mar 1, 2020 at 3:58 AM Bart Van Assche <bvanassche@acm.org> 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.