On 2020-03-02 01:58, Danil Kipnis wrote:
> 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?
Right, if this function is only called during module init there is no
need to worry about concurrency.
Bart.
