2023-05-04 07:56:55

by Peng Fan (OSS)

[permalink] [raw]
Subject: [PATCH] devres: provide API devm_kstrndup

From: Peng Fan <[email protected]>

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

Signed-off-by: Peng Fan <[email protected]>
---
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


2023-05-04 09:13:34

by Greg Kroah-Hartman

[permalink] [raw]
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 <[email protected]>
>
> This patch introduces devm_kstrndup API so that the
> device's driver can allocate memory and copy string.
>
> Signed-off-by: Peng Fan <[email protected]>
> ---
> 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

2023-05-04 09:29:26

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH] devres: provide API devm_kstrndup

On Thu, May 04, 2023 at 08:59:41AM +0000, Peng Fan wrote:
> Hi Greg,
>
> > -----Original Message-----
> > From: Greg KH <[email protected]>
> > Sent: 2023年5月4日 16:58
> > To: Peng Fan (OSS) <[email protected]>
> > Cc: [email protected]; [email protected];
> > [email protected]; [email protected];
> > [email protected]; [email protected]; [email protected];
> > [email protected]; [email protected]; [email protected];
> > [email protected]; Peng Fan <[email protected]>
> > 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 <[email protected]>
> > >
> > > This patch introduces devm_kstrndup API so that the device's driver
> > > can allocate memory and copy string.
> > >
> > > Signed-off-by: Peng Fan <[email protected]>
> > > ---
> > > 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/[email protected]/

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

2023-05-04 09:32:17

by Peng Fan

[permalink] [raw]
Subject: RE: [PATCH] devres: provide API devm_kstrndup

Hi Greg,

> -----Original Message-----
> From: Greg KH <[email protected]>
> Sent: 2023??5??4?? 16:58
> To: Peng Fan (OSS) <[email protected]>
> Cc: [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; Peng Fan <[email protected]>
> 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 <[email protected]>
> >
> > This patch introduces devm_kstrndup API so that the device's driver
> > can allocate memory and copy string.
> >
> > Signed-off-by: Peng Fan <[email protected]>
> > ---
> > 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/[email protected]/

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

2023-05-04 14:41:31

by Andy Shevchenko

[permalink] [raw]
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 <[email protected]>
>
> 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