[v1] rpmsg: Use strscpy() instead of strscpy_pad()

[RESEND PATCH] rpmsg: Use strscpy() instead of strscpy_pad()

Posted by Thorsten Blum 9 months, 2 weeks ago

kzalloc() already zero-initializes the destination buffer, making
strscpy() sufficient for safely copying the name. The additional NUL-
padding performed by strscpy_pad() is unnecessary.

The size parameter is optional, and strscpy() automatically determines
the size of the destination buffer using sizeof() when the argument is
omitted. RPMSG_NAME_SIZE is equal to sizeof(rpdev->id.name) and can be
removed - remove it.

No functional changes intended.

Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
---
 drivers/rpmsg/qcom_glink_native.c | 2 +-
 drivers/rpmsg/qcom_smd.c          | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/rpmsg/qcom_glink_native.c b/drivers/rpmsg/qcom_glink_native.c
index a2f9d85c7156..820a6ca5b1d7 100644
--- a/drivers/rpmsg/qcom_glink_native.c
+++ b/drivers/rpmsg/qcom_glink_native.c
@@ -1663,7 +1663,7 @@ static int qcom_glink_rx_open(struct qcom_glink *glink, unsigned int rcid,
 		}
 
 		rpdev->ept = &channel->ept;
-		strscpy_pad(rpdev->id.name, name, RPMSG_NAME_SIZE);
+		strscpy(rpdev->id.name, name);
 		rpdev->src = RPMSG_ADDR_ANY;
 		rpdev->dst = RPMSG_ADDR_ANY;
 		rpdev->ops = &glink_device_ops;
diff --git a/drivers/rpmsg/qcom_smd.c b/drivers/rpmsg/qcom_smd.c
index 40d386809d6b..3c86c5553de6 100644
--- a/drivers/rpmsg/qcom_smd.c
+++ b/drivers/rpmsg/qcom_smd.c
@@ -1089,7 +1089,7 @@ static int qcom_smd_create_device(struct qcom_smd_channel *channel)
 
 	/* Assign public information to the rpmsg_device */
 	rpdev = &qsdev->rpdev;
-	strscpy_pad(rpdev->id.name, channel->name, RPMSG_NAME_SIZE);
+	strscpy(rpdev->id.name, channel->name);
 	rpdev->src = RPMSG_ADDR_ANY;
 	rpdev->dst = RPMSG_ADDR_ANY;
 
-- 
2.49.0

Re: [RESEND PATCH] rpmsg: Use strscpy() instead of strscpy_pad()

Posted by Bjorn Andersson 7 months, 3 weeks ago

On Tue, 29 Apr 2025 12:45:43 +0200, Thorsten Blum wrote:
> kzalloc() already zero-initializes the destination buffer, making
> strscpy() sufficient for safely copying the name. The additional NUL-
> padding performed by strscpy_pad() is unnecessary.
> 
> The size parameter is optional, and strscpy() automatically determines
> the size of the destination buffer using sizeof() when the argument is
> omitted. RPMSG_NAME_SIZE is equal to sizeof(rpdev->id.name) and can be
> removed - remove it.
> 
> [...]

Applied, thanks!

[1/1] rpmsg: Use strscpy() instead of strscpy_pad()
      commit: 28b825975b8feb352e996d77f679e790b4d84913

Best regards,
-- 
Bjorn Andersson <andersson@kernel.org>

Re: [RESEND PATCH] rpmsg: Use strscpy() instead of strscpy_pad()

Posted by Mathieu Poirier 9 months, 1 week ago

On Tue, 29 Apr 2025 at 04:46, Thorsten Blum <thorsten.blum@linux.dev> wrote:
>
> kzalloc() already zero-initializes the destination buffer, making
> strscpy() sufficient for safely copying the name. The additional NUL-
> padding performed by strscpy_pad() is unnecessary.
>
> The size parameter is optional, and strscpy() automatically determines
> the size of the destination buffer using sizeof() when the argument is
> omitted. RPMSG_NAME_SIZE is equal to sizeof(rpdev->id.name) and can be
> removed - remove it.
>
> No functional changes intended.
>
> Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
> ---
>  drivers/rpmsg/qcom_glink_native.c | 2 +-
>  drivers/rpmsg/qcom_smd.c          | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)


I will let Bjorn take care of this one.

Thanks,
Mathieu

>
> diff --git a/drivers/rpmsg/qcom_glink_native.c b/drivers/rpmsg/qcom_glink_native.c
> index a2f9d85c7156..820a6ca5b1d7 100644
> --- a/drivers/rpmsg/qcom_glink_native.c
> +++ b/drivers/rpmsg/qcom_glink_native.c
> @@ -1663,7 +1663,7 @@ static int qcom_glink_rx_open(struct qcom_glink *glink, unsigned int rcid,
>                 }
>
>                 rpdev->ept = &channel->ept;
> -               strscpy_pad(rpdev->id.name, name, RPMSG_NAME_SIZE);
> +               strscpy(rpdev->id.name, name);
>                 rpdev->src = RPMSG_ADDR_ANY;
>                 rpdev->dst = RPMSG_ADDR_ANY;
>                 rpdev->ops = &glink_device_ops;
> diff --git a/drivers/rpmsg/qcom_smd.c b/drivers/rpmsg/qcom_smd.c
> index 40d386809d6b..3c86c5553de6 100644
> --- a/drivers/rpmsg/qcom_smd.c
> +++ b/drivers/rpmsg/qcom_smd.c
> @@ -1089,7 +1089,7 @@ static int qcom_smd_create_device(struct qcom_smd_channel *channel)
>
>         /* Assign public information to the rpmsg_device */
>         rpdev = &qsdev->rpdev;
> -       strscpy_pad(rpdev->id.name, channel->name, RPMSG_NAME_SIZE);
> +       strscpy(rpdev->id.name, channel->name);
>         rpdev->src = RPMSG_ADDR_ANY;
>         rpdev->dst = RPMSG_ADDR_ANY;
>
> --
> 2.49.0
>

Re: [RESEND PATCH] rpmsg: Use strscpy() instead of strscpy_pad()

Posted by Thorsten Blum 8 months ago

Hi Mathieu,

On 30. Apr 2025, at 16:59, Mathieu Poirier wrote:
> On Tue, 29 Apr 2025 at 04:46, Thorsten Blum wrote:
>> 
>> kzalloc() already zero-initializes the destination buffer, making
>> strscpy() sufficient for safely copying the name. The additional NUL-
>> padding performed by strscpy_pad() is unnecessary.
>> 
>> The size parameter is optional, and strscpy() automatically determines
>> the size of the destination buffer using sizeof() when the argument is
>> omitted. RPMSG_NAME_SIZE is equal to sizeof(rpdev->id.name) and can be
>> removed - remove it.
>> 
>> No functional changes intended.
>> 
>> Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
>> ---
>> drivers/rpmsg/qcom_glink_native.c | 2 +-
>> drivers/rpmsg/qcom_smd.c          | 2 +-
>> 2 files changed, 2 insertions(+), 2 deletions(-)
> 
> 
> I will let Bjorn take care of this one.

This one didn't make it into the last merge window, did it?

Could you or Bjorn take care of it or should I resend it?

Thanks,
Thorsten

Re: [RESEND PATCH] rpmsg: Use strscpy() instead of strscpy_pad()

Posted by Bjorn Andersson 8 months ago

On Tue, Jun 10, 2025 at 12:38:14PM +0200, Thorsten Blum wrote:
> Hi Mathieu,
> 
> On 30. Apr 2025, at 16:59, Mathieu Poirier wrote:
> > On Tue, 29 Apr 2025 at 04:46, Thorsten Blum wrote:
> >> 
> >> kzalloc() already zero-initializes the destination buffer, making
> >> strscpy() sufficient for safely copying the name. The additional NUL-
> >> padding performed by strscpy_pad() is unnecessary.
> >> 
> >> The size parameter is optional, and strscpy() automatically determines
> >> the size of the destination buffer using sizeof() when the argument is
> >> omitted. RPMSG_NAME_SIZE is equal to sizeof(rpdev->id.name) and can be
> >> removed - remove it.
> >> 
> >> No functional changes intended.
> >> 
> >> Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
> >> ---
> >> drivers/rpmsg/qcom_glink_native.c | 2 +-
> >> drivers/rpmsg/qcom_smd.c          | 2 +-
> >> 2 files changed, 2 insertions(+), 2 deletions(-)
> > 
> > 
> > I will let Bjorn take care of this one.
> 
> This one didn't make it into the last merge window, did it?
> 
> Could you or Bjorn take care of it or should I resend it?
> 

I looked at it, but needed to convince myself that there were no side
effects, and then missed pulling it. It's still in the queue, so I'm
taking another look, no need to resend it.

Thanks,
Bjorn

> Thanks,
> Thorsten
>