devres: provide API devm_kstrndup

[PATCH] devres: provide API devm_kstrndup

Posted by Peng Fan (OSS) 2 years, 7 months ago

From: Peng Fan <peng.fan@nxp.com>

This patch introduces devm_kstrndup API so that the
device's driver can allocate memory and copy string.

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/base/devres.c  | 28 ++++++++++++++++++++++++++++
 include/linux/device.h |  1 +
 2 files changed, 29 insertions(+)

diff --git a/drivers/base/devres.c b/drivers/base/devres.c
index 5c998cfac335..87e2965e5bab 100644
--- a/drivers/base/devres.c
+++ b/drivers/base/devres.c
@@ -963,6 +963,34 @@ char *devm_kstrdup(struct device *dev, const char *s, gfp_t gfp)
 }
 EXPORT_SYMBOL_GPL(devm_kstrdup);
 
+/**
+ * devm_kstrndup - Allocate resource managed space and
+ *                copy an existing string into that.
+ * @dev: Device to allocate memory for
+ * @s: the string to duplicate
+ * @max: max length to duplicate
+ * @gfp: the GFP mask used in the devm_kmalloc() call when allocating memory
+ * RETURNS:
+ * Pointer to allocated string on success, NULL on failure.
+ */
+char *devm_kstrndup(struct device *dev, const char *s, size_t max, gfp_t gfp)
+{
+	size_t len;
+	char *buf;
+
+	if (!s)
+		return NULL;
+
+	len = strnlen(s, max);
+	buf = devm_kmalloc(dev, len + 1, gfp);
+	if (buf) {
+		memcpy(buf, s, len);
+		buf[len] = '\0';
+	}
+	return buf;
+}
+EXPORT_SYMBOL_GPL(devm_kstrndup);
+
 /**
  * devm_kstrdup_const - resource managed conditional string duplication
  * @dev: device for which to duplicate the string
diff --git a/include/linux/device.h b/include/linux/device.h
index 472dd24d4823..56b092883c64 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -226,6 +226,7 @@ static inline void *devm_kcalloc(struct device *dev,
 void devm_kfree(struct device *dev, const void *p);
 char *devm_kstrdup(struct device *dev, const char *s, gfp_t gfp) __malloc;
 const char *devm_kstrdup_const(struct device *dev, const char *s, gfp_t gfp);
+char *devm_kstrndup(struct device *dev, const char *s, size_t max, gfp_t gfp) __malloc;
 void *devm_kmemdup(struct device *dev, const void *src, size_t len, gfp_t gfp)
 	__realloc_size(3);
 
-- 
2.37.1

Re: [PATCH] devres: provide API devm_kstrndup

Posted by Andy Shevchenko 2 years, 7 months ago

On Thu, May 04, 2023 at 03:57:54PM +0800, Peng Fan (OSS) wrote:
> From: Peng Fan <peng.fan@nxp.com>
> 
> This patch introduces devm_kstrndup API so that the
> device's driver can allocate memory and copy string.

...

> +char *devm_kstrndup(struct device *dev, const char *s, size_t max, gfp_t gfp)
> +{
> +	size_t len;
> +	char *buf;
> +
> +	if (!s)
> +		return NULL;
> +
> +	len = strnlen(s, max);

> +	buf = devm_kmalloc(dev, len + 1, gfp);
> +	if (buf) {
> +		memcpy(buf, s, len);

besides this to be open coded devm_kmemdup()

> +		buf[len] = '\0';
> +	}
> +	return buf;
> +}

you can always use devm_add_action_or_reset() for a single user before
we introduce a new wide API.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] devres: provide API devm_kstrndup

Posted by Greg KH 2 years, 7 months ago

On Thu, May 04, 2023 at 03:57:54PM +0800, Peng Fan (OSS) wrote:
> From: Peng Fan <peng.fan@nxp.com>
> 
> This patch introduces devm_kstrndup API so that the
> device's driver can allocate memory and copy string.
> 
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
>  drivers/base/devres.c  | 28 ++++++++++++++++++++++++++++
>  include/linux/device.h |  1 +
>  2 files changed, 29 insertions(+)

We can not add apis with no users, please submit this at the same time a
driver needs it.

And what driver would need to copy a string?


> 
> diff --git a/drivers/base/devres.c b/drivers/base/devres.c
> index 5c998cfac335..87e2965e5bab 100644
> --- a/drivers/base/devres.c
> +++ b/drivers/base/devres.c
> @@ -963,6 +963,34 @@ char *devm_kstrdup(struct device *dev, const char *s, gfp_t gfp)
>  }
>  EXPORT_SYMBOL_GPL(devm_kstrdup);
>  
> +/**
> + * devm_kstrndup - Allocate resource managed space and
> + *                copy an existing string into that.
> + * @dev: Device to allocate memory for
> + * @s: the string to duplicate
> + * @max: max length to duplicate
> + * @gfp: the GFP mask used in the devm_kmalloc() call when allocating memory
> + * RETURNS:
> + * Pointer to allocated string on success, NULL on failure.
> + */
> +char *devm_kstrndup(struct device *dev, const char *s, size_t max, gfp_t gfp)
> +{
> +	size_t len;
> +	char *buf;
> +
> +	if (!s)
> +		return NULL;
> +
> +	len = strnlen(s, max);
> +	buf = devm_kmalloc(dev, len + 1, gfp);
> +	if (buf) {
> +		memcpy(buf, s, len);
> +		buf[len] = '\0';

Why not use kstrndup() instead of copying the same logic here?

thanks,

greg k-h

RE: [PATCH] devres: provide API devm_kstrndup

Posted by Peng Fan 2 years, 7 months ago

Hi Greg,

> -----Original Message-----
> From: Greg KH <gregkh@linuxfoundation.org>
> Sent: 2023年5月4日 16:58
> To: Peng Fan (OSS) <peng.fan@oss.nxp.com>
> Cc: robh+dt@kernel.org; krzysztof.kozlowski+dt@linaro.org;
> rafael@kernel.org; andriy.shevchenko@linux.intel.com;
> hdegoede@redhat.com; jgg@ziepe.ca; saravanak@google.com;
> keescook@chromium.org; tglx@linutronix.de; linux-kernel@vger.kernel.org;
> abel.vesa@linaro.org; Peng Fan <peng.fan@nxp.com>
> Subject: Re: [PATCH] devres: provide API devm_kstrndup
> 
> On Thu, May 04, 2023 at 03:57:54PM +0800, Peng Fan (OSS) wrote:
> > From: Peng Fan <peng.fan@nxp.com>
> >
> > This patch introduces devm_kstrndup API so that the device's driver
> > can allocate memory and copy string.
> >
> > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > ---
> >  drivers/base/devres.c  | 28 ++++++++++++++++++++++++++++
> > include/linux/device.h |  1 +
> >  2 files changed, 29 insertions(+)
> 
> We can not add apis with no users, please submit this at the same time a
> driver needs it.
> 
> And what driver would need to copy a string?

https://lore.kernel.org/all/20230504085506.504474-1-peng.fan@oss.nxp.com/

Should I submit V2 with the upper patch in a patchset?

Thanks,
Peng.

> 
> 
> >
> > diff --git a/drivers/base/devres.c b/drivers/base/devres.c index
> > 5c998cfac335..87e2965e5bab 100644
> > --- a/drivers/base/devres.c
> > +++ b/drivers/base/devres.c
> > @@ -963,6 +963,34 @@ char *devm_kstrdup(struct device *dev, const
> char
> > *s, gfp_t gfp)  }  EXPORT_SYMBOL_GPL(devm_kstrdup);
> >
> > +/**
> > + * devm_kstrndup - Allocate resource managed space and
> > + *                copy an existing string into that.
> > + * @dev: Device to allocate memory for
> > + * @s: the string to duplicate
> > + * @max: max length to duplicate
> > + * @gfp: the GFP mask used in the devm_kmalloc() call when allocating
> > +memory
> > + * RETURNS:
> > + * Pointer to allocated string on success, NULL on failure.
> > + */
> > +char *devm_kstrndup(struct device *dev, const char *s, size_t max,
> > +gfp_t gfp) {
> > +	size_t len;
> > +	char *buf;
> > +
> > +	if (!s)
> > +		return NULL;
> > +
> > +	len = strnlen(s, max);
> > +	buf = devm_kmalloc(dev, len + 1, gfp);
> > +	if (buf) {
> > +		memcpy(buf, s, len);
> > +		buf[len] = '\0';
> 
> Why not use kstrndup() instead of copying the same logic here?
> 
> thanks,
> 
> greg k-h

Re: [PATCH] devres: provide API devm_kstrndup

Posted by Greg KH 2 years, 7 months ago

On Thu, May 04, 2023 at 08:59:41AM +0000, Peng Fan wrote:
> Hi Greg,
> 
> > -----Original Message-----
> > From: Greg KH <gregkh@linuxfoundation.org>
> > Sent: 2023年5月4日 16:58
> > To: Peng Fan (OSS) <peng.fan@oss.nxp.com>
> > Cc: robh+dt@kernel.org; krzysztof.kozlowski+dt@linaro.org;
> > rafael@kernel.org; andriy.shevchenko@linux.intel.com;
> > hdegoede@redhat.com; jgg@ziepe.ca; saravanak@google.com;
> > keescook@chromium.org; tglx@linutronix.de; linux-kernel@vger.kernel.org;
> > abel.vesa@linaro.org; Peng Fan <peng.fan@nxp.com>
> > Subject: Re: [PATCH] devres: provide API devm_kstrndup
> > 
> > On Thu, May 04, 2023 at 03:57:54PM +0800, Peng Fan (OSS) wrote:
> > > From: Peng Fan <peng.fan@nxp.com>
> > >
> > > This patch introduces devm_kstrndup API so that the device's driver
> > > can allocate memory and copy string.
> > >
> > > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > > ---
> > >  drivers/base/devres.c  | 28 ++++++++++++++++++++++++++++
> > > include/linux/device.h |  1 +
> > >  2 files changed, 29 insertions(+)
> > 
> > We can not add apis with no users, please submit this at the same time a
> > driver needs it.
> > 
> > And what driver would need to copy a string?
> 
> https://lore.kernel.org/all/20230504085506.504474-1-peng.fan@oss.nxp.com/

Please never add new module parameters, ESPECIALLY for a driver.  This
is not the 1990's anymore, please use the correct apis instead (hint,
sysfs, configfs, DT, etc.)

> Should I submit V2 with the upper patch in a patchset?

I can't take this as-is, for the reason I said, so yes.  And again, your
dependant patch is not ok either, so I don't think this is needed
anymore.

thanks,

greg k-h