| Message ID | 20240828030321.20688-7-laoar.shao@gmail.com (mailing list archive) |
|---|---|
| State | Handled Elsewhere |
| Delegated to: | Paul Moore |
| Headers | show |
| Series | Improve the copy of task comm | expand |
On Wed, Aug 28, 2024 at 11:03:19AM GMT, Yafang Shao wrote: > These three functions follow the same pattern. To deduplicate the code, > let's introduce a common helper __kmemdup_nul(). > > Suggested-by: Andrew Morton <akpm@linux-foundation.org> > Signed-off-by: Yafang Shao <laoar.shao@gmail.com> > Cc: Simon Horman <horms@kernel.org> > Cc: Matthew Wilcox <willy@infradead.org> > Cc: Alejandro Colomar <alx@kernel.org> > --- Reviewed-by: Alejandro Colomar <alx@kernel.org> Cheers, Alex > mm/util.c | 68 ++++++++++++++++++++++--------------------------------- > 1 file changed, 27 insertions(+), 41 deletions(-) > > diff --git a/mm/util.c b/mm/util.c > index 9a77a347c385..42714fe13e24 100644 > --- a/mm/util.c > +++ b/mm/util.c > @@ -45,33 +45,41 @@ void kfree_const(const void *x) > EXPORT_SYMBOL(kfree_const); > > /** > - * kstrdup - allocate space for and copy an existing string > - * @s: the string to duplicate > + * __kmemdup_nul - Create a NUL-terminated string from @s, which might be unterminated. > + * @s: The data to copy > + * @len: The size of the data, not including the NUL terminator > * @gfp: the GFP mask used in the kmalloc() call when allocating memory > * > - * Return: newly allocated copy of @s or %NULL in case of error > + * Return: newly allocated copy of @s with NUL-termination or %NULL in > + * case of error > */ > -noinline > -char *kstrdup(const char *s, gfp_t gfp) > +static __always_inline char *__kmemdup_nul(const char *s, size_t len, gfp_t gfp) > { > - size_t len; > char *buf; > > - if (!s) > + /* '+1' for the NUL terminator */ > + buf = kmalloc_track_caller(len + 1, gfp); > + if (!buf) > return NULL; > > - len = strlen(s) + 1; > - buf = kmalloc_track_caller(len, gfp); > - if (buf) { > - memcpy(buf, s, len); > - /* During memcpy(), the string might be updated to a new value, > - * which could be longer than the string when strlen() is > - * called. Therefore, we need to add a NUL termimator. > - */ > - buf[len - 1] = '\0'; > - } > + memcpy(buf, s, len); > + /* Ensure the buf is always NUL-terminated, regardless of @s. */ > + buf[len] = '\0'; > return buf; > } > + > +/** > + * kstrdup - allocate space for and copy an existing string > + * @s: the string to duplicate > + * @gfp: the GFP mask used in the kmalloc() call when allocating memory > + * > + * Return: newly allocated copy of @s or %NULL in case of error > + */ > +noinline > +char *kstrdup(const char *s, gfp_t gfp) > +{ > + return s ? __kmemdup_nul(s, strlen(s), gfp) : NULL; > +} > EXPORT_SYMBOL(kstrdup); > > /** > @@ -106,19 +114,7 @@ EXPORT_SYMBOL(kstrdup_const); > */ > char *kstrndup(const char *s, size_t max, gfp_t gfp) > { > - size_t len; > - char *buf; > - > - if (!s) > - return NULL; > - > - len = strnlen(s, max); > - buf = kmalloc_track_caller(len+1, gfp); > - if (buf) { > - memcpy(buf, s, len); > - buf[len] = '\0'; > - } > - return buf; > + return s ? __kmemdup_nul(s, strnlen(s, max), gfp) : NULL; > } > EXPORT_SYMBOL(kstrndup); > > @@ -192,17 +188,7 @@ EXPORT_SYMBOL(kvmemdup); > */ > char *kmemdup_nul(const char *s, size_t len, gfp_t gfp) > { > - char *buf; > - > - if (!s) > - return NULL; > - > - buf = kmalloc_track_caller(len + 1, gfp); > - if (buf) { > - memcpy(buf, s, len); > - buf[len] = '\0'; > - } > - return buf; > + return s ? __kmemdup_nul(s, len, gfp) : NULL; > } > EXPORT_SYMBOL(kmemdup_nul); > > -- > 2.43.5 >
On Wed, Aug 28, 2024 at 12:32:53PM GMT, Alejandro Colomar wrote: > On Wed, Aug 28, 2024 at 11:03:19AM GMT, Yafang Shao wrote: > > These three functions follow the same pattern. To deduplicate the code, > > let's introduce a common helper __kmemdup_nul(). > > > > Suggested-by: Andrew Morton <akpm@linux-foundation.org> > > Signed-off-by: Yafang Shao <laoar.shao@gmail.com> > > Cc: Simon Horman <horms@kernel.org> > > Cc: Matthew Wilcox <willy@infradead.org> > > Cc: Alejandro Colomar <alx@kernel.org> > > --- > > Reviewed-by: Alejandro Colomar <alx@kernel.org> (Or maybe I should say Co-developed-by: Alejandro Colomar <alx@kernel.org> Signed-off-by: Alejandro Colomar <alx@kernel.org> ) > > Cheers, > Alex > > > mm/util.c | 68 ++++++++++++++++++++++--------------------------------- > > 1 file changed, 27 insertions(+), 41 deletions(-) > > > > diff --git a/mm/util.c b/mm/util.c > > index 9a77a347c385..42714fe13e24 100644 > > --- a/mm/util.c > > +++ b/mm/util.c > > @@ -45,33 +45,41 @@ void kfree_const(const void *x) > > EXPORT_SYMBOL(kfree_const); > > > > /** > > - * kstrdup - allocate space for and copy an existing string > > - * @s: the string to duplicate > > + * __kmemdup_nul - Create a NUL-terminated string from @s, which might be unterminated. > > + * @s: The data to copy > > + * @len: The size of the data, not including the NUL terminator > > * @gfp: the GFP mask used in the kmalloc() call when allocating memory > > * > > - * Return: newly allocated copy of @s or %NULL in case of error > > + * Return: newly allocated copy of @s with NUL-termination or %NULL in > > + * case of error > > */ > > -noinline > > -char *kstrdup(const char *s, gfp_t gfp) > > +static __always_inline char *__kmemdup_nul(const char *s, size_t len, gfp_t gfp) > > { > > - size_t len; > > char *buf; > > > > - if (!s) > > + /* '+1' for the NUL terminator */ > > + buf = kmalloc_track_caller(len + 1, gfp); > > + if (!buf) > > return NULL; > > > > - len = strlen(s) + 1; > > - buf = kmalloc_track_caller(len, gfp); > > - if (buf) { > > - memcpy(buf, s, len); > > - /* During memcpy(), the string might be updated to a new value, > > - * which could be longer than the string when strlen() is > > - * called. Therefore, we need to add a NUL termimator. > > - */ > > - buf[len - 1] = '\0'; > > - } > > + memcpy(buf, s, len); > > + /* Ensure the buf is always NUL-terminated, regardless of @s. */ > > + buf[len] = '\0'; > > return buf; > > } > > + > > +/** > > + * kstrdup - allocate space for and copy an existing string > > + * @s: the string to duplicate > > + * @gfp: the GFP mask used in the kmalloc() call when allocating memory > > + * > > + * Return: newly allocated copy of @s or %NULL in case of error > > + */ > > +noinline > > +char *kstrdup(const char *s, gfp_t gfp) > > +{ > > + return s ? __kmemdup_nul(s, strlen(s), gfp) : NULL; > > +} > > EXPORT_SYMBOL(kstrdup); > > > > /** > > @@ -106,19 +114,7 @@ EXPORT_SYMBOL(kstrdup_const); > > */ > > char *kstrndup(const char *s, size_t max, gfp_t gfp) > > { > > - size_t len; > > - char *buf; > > - > > - if (!s) > > - return NULL; > > - > > - len = strnlen(s, max); > > - buf = kmalloc_track_caller(len+1, gfp); > > - if (buf) { > > - memcpy(buf, s, len); > > - buf[len] = '\0'; > > - } > > - return buf; > > + return s ? __kmemdup_nul(s, strnlen(s, max), gfp) : NULL; > > } > > EXPORT_SYMBOL(kstrndup); > > > > @@ -192,17 +188,7 @@ EXPORT_SYMBOL(kvmemdup); > > */ > > char *kmemdup_nul(const char *s, size_t len, gfp_t gfp) > > { > > - char *buf; > > - > > - if (!s) > > - return NULL; > > - > > - buf = kmalloc_track_caller(len + 1, gfp); > > - if (buf) { > > - memcpy(buf, s, len); > > - buf[len] = '\0'; > > - } > > - return buf; > > + return s ? __kmemdup_nul(s, len, gfp) : NULL; > > } > > EXPORT_SYMBOL(kmemdup_nul); > > > > -- > > 2.43.5 > > > > -- > <https://www.alejandro-colomar.es/>
On Wed, Aug 28, 2024 at 6:33 PM Alejandro Colomar <alx@kernel.org> wrote: > > On Wed, Aug 28, 2024 at 12:32:53PM GMT, Alejandro Colomar wrote: > > On Wed, Aug 28, 2024 at 11:03:19AM GMT, Yafang Shao wrote: > > > These three functions follow the same pattern. To deduplicate the code, > > > let's introduce a common helper __kmemdup_nul(). > > > > > > Suggested-by: Andrew Morton <akpm@linux-foundation.org> > > > Signed-off-by: Yafang Shao <laoar.shao@gmail.com> > > > Cc: Simon Horman <horms@kernel.org> > > > Cc: Matthew Wilcox <willy@infradead.org> > > > Cc: Alejandro Colomar <alx@kernel.org> > > > --- > > > > Reviewed-by: Alejandro Colomar <alx@kernel.org> > > (Or maybe I should say > > Co-developed-by: Alejandro Colomar <alx@kernel.org> > Signed-off-by: Alejandro Colomar <alx@kernel.org> I will add it. Thanks for your help.
diff --git a/mm/util.c b/mm/util.c index 9a77a347c385..42714fe13e24 100644 --- a/mm/util.c +++ b/mm/util.c @@ -45,33 +45,41 @@ void kfree_const(const void *x) EXPORT_SYMBOL(kfree_const); /** - * kstrdup - allocate space for and copy an existing string - * @s: the string to duplicate + * __kmemdup_nul - Create a NUL-terminated string from @s, which might be unterminated. + * @s: The data to copy + * @len: The size of the data, not including the NUL terminator * @gfp: the GFP mask used in the kmalloc() call when allocating memory * - * Return: newly allocated copy of @s or %NULL in case of error + * Return: newly allocated copy of @s with NUL-termination or %NULL in + * case of error */ -noinline -char *kstrdup(const char *s, gfp_t gfp) +static __always_inline char *__kmemdup_nul(const char *s, size_t len, gfp_t gfp) { - size_t len; char *buf; - if (!s) + /* '+1' for the NUL terminator */ + buf = kmalloc_track_caller(len + 1, gfp); + if (!buf) return NULL; - len = strlen(s) + 1; - buf = kmalloc_track_caller(len, gfp); - if (buf) { - memcpy(buf, s, len); - /* During memcpy(), the string might be updated to a new value, - * which could be longer than the string when strlen() is - * called. Therefore, we need to add a NUL termimator. - */ - buf[len - 1] = '\0'; - } + memcpy(buf, s, len); + /* Ensure the buf is always NUL-terminated, regardless of @s. */ + buf[len] = '\0'; return buf; } + +/** + * kstrdup - allocate space for and copy an existing string + * @s: the string to duplicate + * @gfp: the GFP mask used in the kmalloc() call when allocating memory + * + * Return: newly allocated copy of @s or %NULL in case of error + */ +noinline +char *kstrdup(const char *s, gfp_t gfp) +{ + return s ? __kmemdup_nul(s, strlen(s), gfp) : NULL; +} EXPORT_SYMBOL(kstrdup); /** @@ -106,19 +114,7 @@ EXPORT_SYMBOL(kstrdup_const); */ char *kstrndup(const char *s, size_t max, gfp_t gfp) { - size_t len; - char *buf; - - if (!s) - return NULL; - - len = strnlen(s, max); - buf = kmalloc_track_caller(len+1, gfp); - if (buf) { - memcpy(buf, s, len); - buf[len] = '\0'; - } - return buf; + return s ? __kmemdup_nul(s, strnlen(s, max), gfp) : NULL; } EXPORT_SYMBOL(kstrndup); @@ -192,17 +188,7 @@ EXPORT_SYMBOL(kvmemdup); */ char *kmemdup_nul(const char *s, size_t len, gfp_t gfp) { - char *buf; - - if (!s) - return NULL; - - buf = kmalloc_track_caller(len + 1, gfp); - if (buf) { - memcpy(buf, s, len); - buf[len] = '\0'; - } - return buf; + return s ? __kmemdup_nul(s, len, gfp) : NULL; } EXPORT_SYMBOL(kmemdup_nul);
These three functions follow the same pattern. To deduplicate the code, let's introduce a common helper __kmemdup_nul(). Suggested-by: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Yafang Shao <laoar.shao@gmail.com> Cc: Simon Horman <horms@kernel.org> Cc: Matthew Wilcox <willy@infradead.org> Cc: Alejandro Colomar <alx@kernel.org> --- mm/util.c | 68 ++++++++++++++++++++++--------------------------------- 1 file changed, 27 insertions(+), 41 deletions(-)