Messages in this thread | | | Subject | Re: [PATCH 02/13] staging: greybus: Switch from strlcpy to strscpy | From | Alex Elder <> | Date | Tue, 16 Feb 2021 08:54:59 -0600 |
| |
On 1/31/21 11:28 AM, Kumar Kartikeya Dwivedi wrote: > strlcpy is marked as deprecated in Documentation/process/deprecated.rst, > and there is no functional difference when the caller expects truncation > (when not checking the return value). strscpy is relatively better as it > also avoids scanning the whole source string. > > This silences the related checkpatch warnings from: > 5dbdb2d87c29 ("checkpatch: prefer strscpy to strlcpy") > > Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
This is a good change. But while you're at it, I would appreciate if you would convert a few spots to use sizeof(dest) rather than a fixed constant. I will point them out below.
If this is the *only* request for a change on your series, please tell me that and I can sign off on this without you implementing my suggestion. But if you post a version 2, please do what I describe.
Thanks.
-Alex
> --- > drivers/staging/greybus/audio_helper.c | 2 +- > drivers/staging/greybus/audio_module.c | 2 +- > drivers/staging/greybus/audio_topology.c | 6 +++--- > drivers/staging/greybus/power_supply.c | 2 +- > drivers/staging/greybus/spilib.c | 4 ++-- > 5 files changed, 8 insertions(+), 8 deletions(-)
. . .
> diff --git a/drivers/staging/greybus/audio_module.c b/drivers/staging/greybus/audio_module.c > index a243d60f0..0f9fdc077 100644 > --- a/drivers/staging/greybus/audio_module.c > +++ b/drivers/staging/greybus/audio_module.c > @@ -342,7 +342,7 @@ static int gb_audio_probe(struct gb_bundle *bundle, > /* inform above layer for uevent */ > dev_dbg(dev, "Inform set_event:%d to above layer\n", 1); > /* prepare for the audio manager */ > - strlcpy(desc.name, gbmodule->name, GB_AUDIO_MANAGER_MODULE_NAME_LEN); > + strscpy(desc.name, gbmodule->name, GB_AUDIO_MANAGER_MODULE_NAME_LEN);
Please use this here instead:
strscpy(desc.name, gbmodule->name, sizeof(desc.name));
> desc.vid = 2; /* todo */ > desc.pid = 3; /* todo */ > desc.intf_id = gbmodule->dev_id; > diff --git a/drivers/staging/greybus/audio_topology.c b/drivers/staging/greybus/audio_topology.c > index 662e3e8b4..e816e4db5 100644 > --- a/drivers/staging/greybus/audio_topology.c > +++ b/drivers/staging/greybus/audio_topology.c > @@ -200,7 +200,7 @@ static int gbcodec_mixer_ctl_info(struct snd_kcontrol *kcontrol, > return -EINVAL; > name = gbaudio_map_controlid(module, data->ctl_id, > uinfo->value.enumerated.item); > - strlcpy(uinfo->value.enumerated.name, name, NAME_SIZE); > + strscpy(uinfo->value.enumerated.name, name, NAME_SIZE);
Please use this here instead:
strscpy(uinfo->value.enumerated.name, name, sizeof(uinfo->valiue.enumerated.name));
(I know NAME_SIZE is used throughout this file, and could also be converted in this way, but we can save that for another patch.)
> break; > default: > dev_err(comp->dev, "Invalid type: %d for %s:kcontrol\n", > @@ -1047,7 +1047,7 @@ static int gbaudio_tplg_create_widget(struct gbaudio_module_info *module, > } > > /* Prefix dev_id to widget control_name */ > - strlcpy(temp_name, w->name, NAME_SIZE); > + strscpy(temp_name, w->name, NAME_SIZE);
Please use this here instead:
strscpy(temp_name, w->name, sizeof(temp_name));
> snprintf(w->name, NAME_SIZE, "GB %d %s", module->dev_id, temp_name); > > switch (w->type) { > @@ -1169,7 +1169,7 @@ static int gbaudio_tplg_process_kcontrols(struct gbaudio_module_info *module, > } > control->id = curr->id; > /* Prefix dev_id to widget_name */ > - strlcpy(temp_name, curr->name, NAME_SIZE); > + strscpy(temp_name, curr->name, NAME_SIZE);
Please use this here instead:
strscpy(temp_name, curr->name, sizeof(temp_name));
> snprintf(curr->name, NAME_SIZE, "GB %d %s", module->dev_id, > temp_name); > control->name = curr->name;
. . .
| |