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 <[email protected]>
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;
. . .
Instead, depend on the size of the destination buffer for easier
refactoring.
Signed-off-by: Kumar Kartikeya Dwivedi <[email protected]>
---
Hopefully, this is more thorough. The only cases left now are where the
destination string is represented by a pointer, otherwise all call sites with a
fixed sized buffer have been changed.
---
drivers/staging/greybus/audio_module.c | 4 ++--
drivers/staging/greybus/audio_topology.c | 12 ++++++------
2 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/staging/greybus/audio_module.c b/drivers/staging/greybus/audio_module.c
index 0f9fdc077..12c376c47 100644
--- a/drivers/staging/greybus/audio_module.c
+++ b/drivers/staging/greybus/audio_module.c
@@ -260,7 +260,7 @@ static int gb_audio_probe(struct gb_bundle *bundle,
INIT_LIST_HEAD(&gbmodule->widget_ctl_list);
INIT_LIST_HEAD(&gbmodule->jack_list);
gbmodule->dev = dev;
- snprintf(gbmodule->name, NAME_SIZE, "%s.%s", dev->driver->name,
+ snprintf(gbmodule->name, sizeof(gbmodule->name), "%s.%s", dev->driver->name,
dev_name(dev));
greybus_set_drvdata(bundle, gbmodule);
@@ -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 */
- strscpy(desc.name, gbmodule->name, GB_AUDIO_MANAGER_MODULE_NAME_LEN);
+ 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 e816e4db5..1fc7727ab 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);
- strscpy(uinfo->value.enumerated.name, name, NAME_SIZE);
+ strscpy(uinfo->value.enumerated.name, name, sizeof(uinfo->value.enumerated.name));
break;
default:
dev_err(comp->dev, "Invalid type: %d for %s:kcontrol\n",
@@ -363,7 +363,7 @@ static int gbcodec_mixer_dapm_ctl_info(struct snd_kcontrol *kcontrol,
platform_min = le32_to_cpu(info->value.integer.min);
if (platform_max == 1 &&
- !strnstr(kcontrol->id.name, " Volume", NAME_SIZE))
+ !strnstr(kcontrol->id.name, " Volume", sizeof(kcontrol->id.name)))
uinfo->type = SNDRV_CTL_ELEM_TYPE_BOOLEAN;
else
uinfo->type = SNDRV_CTL_ELEM_TYPE_INTEGER;
@@ -1047,8 +1047,8 @@ static int gbaudio_tplg_create_widget(struct gbaudio_module_info *module,
}
/* Prefix dev_id to widget control_name */
- strscpy(temp_name, w->name, NAME_SIZE);
- snprintf(w->name, NAME_SIZE, "GB %d %s", module->dev_id, temp_name);
+ strscpy(temp_name, w->name, sizeof(temp_name));
+ snprintf(w->name, sizeof(w->name), "GB %d %s", module->dev_id, temp_name);
switch (w->type) {
case snd_soc_dapm_spk:
@@ -1169,8 +1169,8 @@ static int gbaudio_tplg_process_kcontrols(struct gbaudio_module_info *module,
}
control->id = curr->id;
/* Prefix dev_id to widget_name */
- strscpy(temp_name, curr->name, NAME_SIZE);
- snprintf(curr->name, NAME_SIZE, "GB %d %s", module->dev_id,
+ strscpy(temp_name, curr->name, sizeof(temp_name));
+ snprintf(curr->name, sizeof(curr->name), "GB %d %s", module->dev_id,
temp_name);
control->name = curr->name;
if (curr->info.type == GB_AUDIO_CTL_ELEM_TYPE_ENUMERATED) {
--
2.29.2