On Fri, Jun 21, 2024 at 9:51 PM Simon Horman [off-list ref] wrote:

On Fri, Jun 21, 2024 at 10:29:54AM +0800, Yafang Shao wrote:

quoted

These three functions follow the same pattern. To deduplicate the code,
let's introduce a common help __kstrndup().

Suggested-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Yafang Shao <redacted>

Hi Yafang Shao,

Some minor nits from my side.

quoted

---
 mm/internal.h | 24 ++++++++++++++++++++++++
 mm/util.c     | 27 ++++-----------------------
 2 files changed, 28 insertions(+), 23 deletions(-)

diff --git a/mm/internal.h b/mm/internal.h
index b2c75b12014e..fd87f685739b 100644
--- a/mm/internal.h
+++ b/mm/internal.h

@@ -1521,4 +1521,28 @@ static inline void shrinker_debugfs_remove(struct dentry *debugfs_entry,
 void workingset_update_node(struct xa_node *node);
 extern struct list_lru shadow_nodes;

+/**
+ * __kstrndup - Create a NUL-terminated string from @s, which might be unterminated.
+ * @s: The data to stringify
+ * @len: The size of the data, including the null terminator
+ * @gfp: the GFP mask used in the kmalloc() call when allocating memory
+ *
+ * Return: newly allocated copy of @s with NUL-termination or %NULL in
+ * case of error
+ */
+static __always_inline char *__kstrndup(const char *s, size_t len, gfp_t gfp)
+{
+     char *buf;
+
+     buf = kmalloc_track_caller(len, gfp);
+     if (!buf)
+             return NULL;
+
+     memcpy(buf, s, len);
+     /* Ensure the buf is always NUL-terminated, regardless of @s. */
+     buf[len - 1] = '\0';
+     return buf;
+}
+
+

nit: One blank line is enough.

Ah, will change it.

quoted

 #endif       /* __MM_INTERNAL_H */

diff --git a/mm/util.c b/mm/util.c
index 41c7875572ed..d9135c5fdf7f 100644
--- a/mm/util.c
+++ b/mm/util.c

@@ -58,17 +58,8 @@ char *kstrdup(const char *s, gfp_t gfp)
      if (!s)
              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 null termimator.
-              */
-             buf[len - 1] = '\0';
-     }
-     return buf;

nit: The local variable buf is now unused, and should be removed from kstrdup().
     Likewise for kstrndup() and kmemdup_nul()

     Flagged by W=1 builds with gcc-13 and clang-18, and Smatch.

I missed that. Thanks for pointing it out.

-- 
Regards
Yafang