[PATCH v2 0/7] support '%pd' and '%pD' for print file name

[PATCH v2 0/7] support '%pd' and '%pD' for print file name

[Ye Bin]

During fault locating, the file name needs to be printed based on the
dentry/file address. The offset needs to be calculated each time, which
is troublesome. Similar to printk, kprobe supports printing file names
for dentry/file addresses.

Diff v2 vs v1:
1. Use "%pd/%pD" print format instead of "pd/pD" print format;
2. Add "%pd/%pD" in README;
3. Expand "%pd/%pD" argument before parameter parsing;
4. Add more detail information in ftrace documentation;
5. Add test cases for new print format in selftests/ftrace;

Ye Bin (7):
  string.h: add str_has_suffix() helper for test string ends with
    specify string
  tracing/probes: add traceprobe_expand_dentry_args() helper
  tracing/probes: support '%pd' type for print struct dentry's name
  tracing/probes: support '%pD' type for print struct file's name
  tracing: add new type "%pd/%pD" in readme_msg[]
  Documentation: tracing: add new type '%pd' and '%pD' for kprobe
  selftests/ftrace: add test cases for VFS type "%pd" and "%pD"

 Documentation/trace/kprobetrace.rst           |  6 +-
 include/linux/string.h                        | 20 +++++
 kernel/trace/trace.c                          |  2 +-
 kernel/trace/trace_kprobe.c                   |  6 ++
 kernel/trace/trace_probe.c                    | 45 +++++++++++
 kernel/trace/trace_probe.h                    |  3 +
 .../ftrace/test.d/kprobe/kprobe_args_vfs.tc   | 79 +++++++++++++++++++
 7 files changed, 159 insertions(+), 2 deletions(-)
 create mode 100644 tools/testing/selftests/ftrace/test.d/kprobe/kprobe_args_vfs.tc

-- 
2.31.1

[PATCH v2 1/7] string.h: add str_has_suffix() helper for test string ends with specify string

[Ye Bin]

str_has_suffix() is test string if ends with specify string.

Signed-off-by: Ye Bin <yebin10@huawei.com>
---
 include/linux/string.h | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/include/linux/string.h b/include/linux/string.h
index 433c207a01da..e47e9597af27 100644
--- a/include/linux/string.h
+++ b/include/linux/string.h
@@ -405,4 +405,24 @@ static __always_inline size_t str_has_prefix(const char *str, const char *prefix
 	return strncmp(str, prefix, len) == 0 ? len : 0;
 }
 
+/**
+ * str_has_suffix - Test if a string has a given suffix
+ * @str: The string to test
+ * @suffix: The string to see if @str ends with
+ *
+ * Returns:
+ * * strlen(@suffix) if @str ends with @suffix
+ * * 0 if @str does not end with @suffix
+ */
+static __always_inline size_t str_has_suffix(const char *str, const char *suffix)
+{
+	size_t len = strlen(suffix);
+	size_t str_len = strlen(str);
+
+	if (len > str_len)
+		return 0;
+
+	return strncmp(str + str_len - len, suffix, len) == 0 ? len : 0;
+}
+
 #endif /* _LINUX_STRING_H_ */
-- 
2.31.1

[PATCH v2 2/7] tracing/probes: add traceprobe_expand_dentry_args() helper

[Ye Bin]

Add traceprobe_expand_dentry_args() to expand dentry args. this API is
prepare to support "%pd" print format for kprobe.

Signed-off-by: Ye Bin <yebin10@huawei.com>
---
 kernel/trace/trace_probe.c | 34 ++++++++++++++++++++++++++++++++++
 kernel/trace/trace_probe.h |  2 ++
 2 files changed, 36 insertions(+)

diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c
index 4dc74d73fc1d..1599c0c3e6b7 100644
--- a/kernel/trace/trace_probe.c
+++ b/kernel/trace/trace_probe.c
@@ -1565,6 +1565,40 @@ const char **traceprobe_expand_meta_args(int argc, const char *argv[],
 	return ERR_PTR(ret);
 }
 
+int traceprobe_expand_dentry_args(int argc, const char *argv[], char *buf,
+				  int bufsize)
+{
+	int i, used, ret;
+
+	used = 0;
+	for (i = 0; i < argc; i++) {
+		if (str_has_suffix(argv[i], ":%pd")) {
+			char *tmp = kstrdup(argv[i], GFP_KERNEL);
+			char *equal;
+
+			if (!tmp)
+				return -ENOMEM;
+
+			equal = strchr(tmp, '=');
+			if (equal)
+				*equal = '\0';
+			tmp[strlen(argv[i]) - 4] = '\0';
+			ret = snprintf(buf + used, bufsize - used,
+				       "%s%s+0x0(+0x%zx(%s)):string",
+				       equal ? tmp : "", equal ? "=" : "",
+				       offsetof(struct dentry, d_name.name),
+				       equal ? equal + 1 : tmp);
+			kfree(tmp);
+			if (ret >= bufsize - used)
+				return -ENOMEM;
+			argv[i] = buf + used;
+			used += ret + 1;
+		}
+	}
+
+	return 0;
+}
+
 void traceprobe_finish_parse(struct traceprobe_parse_context *ctx)
 {
 	clear_btf_context(ctx);
diff --git a/kernel/trace/trace_probe.h b/kernel/trace/trace_probe.h
index 850d9ecb6765..553371a4e0b1 100644
--- a/kernel/trace/trace_probe.h
+++ b/kernel/trace/trace_probe.h
@@ -402,6 +402,8 @@ extern int traceprobe_parse_probe_arg(struct trace_probe *tp, int i,
 const char **traceprobe_expand_meta_args(int argc, const char *argv[],
 					 int *new_argc, char *buf, int bufsize,
 					 struct traceprobe_parse_context *ctx);
+extern int traceprobe_expand_dentry_args(int argc, const char *argv[], char *buf,
+					 int bufsize);
 
 extern int traceprobe_update_arg(struct probe_arg *arg);
 extern void traceprobe_free_probe_arg(struct probe_arg *arg);
-- 
2.31.1

[PATCH v2 3/7] tracing/probes: support '%pd' type for print struct dentry's name

[Ye Bin]

Similar to '%pd' for printk, use '%pd' for print struct dentry's name.

Signed-off-by: Ye Bin <yebin10@huawei.com>
---
 kernel/trace/trace_kprobe.c | 6 ++++++
 kernel/trace/trace_probe.h  | 1 +
 2 files changed, 7 insertions(+)

diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index c4c6e0e0068b..00b74530fbad 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -779,6 +779,7 @@ static int __trace_kprobe_create(int argc, const char *argv[])
 	char buf[MAX_EVENT_NAME_LEN];
 	char gbuf[MAX_EVENT_NAME_LEN];
 	char abuf[MAX_BTF_ARGS_LEN];
+	char dbuf[MAX_DENTRY_ARGS_LEN];
 	struct traceprobe_parse_context ctx = { .flags = TPARG_FL_KERNEL };
 
 	switch (argv[0][0]) {
@@ -930,6 +931,11 @@ static int __trace_kprobe_create(int argc, const char *argv[])
 		argv = new_argv;
 	}
 
+	ret = traceprobe_expand_dentry_args(argc, argv, dbuf,
+					    MAX_DENTRY_ARGS_LEN);
+	if (ret)
+		goto out;
+
 	/* setup a probe */
 	tk = alloc_trace_kprobe(group, event, addr, symbol, offset, maxactive,
 				argc, is_return);
diff --git a/kernel/trace/trace_probe.h b/kernel/trace/trace_probe.h
index 553371a4e0b1..d9c053824975 100644
--- a/kernel/trace/trace_probe.h
+++ b/kernel/trace/trace_probe.h
@@ -34,6 +34,7 @@
 #define MAX_ARRAY_LEN		64
 #define MAX_ARG_NAME_LEN	32
 #define MAX_BTF_ARGS_LEN	128
+#define MAX_DENTRY_ARGS_LEN	256
 #define MAX_STRING_SIZE		PATH_MAX
 #define MAX_ARG_BUF_LEN		(MAX_TRACE_ARGS * MAX_ARG_NAME_LEN)
 
-- 
2.31.1

[PATCH v2 4/7] tracing/probes: support '%pD' type for print struct file's name

[Ye Bin]

Similar to '%pD' for printk, use '%pD' for print struct file's name.

Signed-off-by: Ye Bin <yebin10@huawei.com>
---
 kernel/trace/trace_probe.c | 41 ++++++++++++++++++++++++--------------
 1 file changed, 26 insertions(+), 15 deletions(-)

diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c
index 1599c0c3e6b7..f9819625de58 100644
--- a/kernel/trace/trace_probe.c
+++ b/kernel/trace/trace_probe.c
@@ -12,6 +12,7 @@
 #define pr_fmt(fmt)	"trace_probe: " fmt
 
 #include <linux/bpf.h>
+#include <linux/fs.h>
 #include "trace_btf.h"
 
 #include "trace_probe.h"
@@ -1572,28 +1573,38 @@ int traceprobe_expand_dentry_args(int argc, const char *argv[], char *buf,
 
 	used = 0;
 	for (i = 0; i < argc; i++) {
-		if (str_has_suffix(argv[i], ":%pd")) {
-			char *tmp = kstrdup(argv[i], GFP_KERNEL);
-			char *equal;
+		if (!str_has_suffix(argv[i], ":%pd") &&
+		    !str_has_suffix(argv[i], ":%pD"))
+			continue;
 
-			if (!tmp)
-				return -ENOMEM;
+		char *tmp = kstrdup(argv[i], GFP_KERNEL);
+		char *equal;
+
+		if (!tmp)
+			return -ENOMEM;
 
-			equal = strchr(tmp, '=');
-			if (equal)
-				*equal = '\0';
-			tmp[strlen(argv[i]) - 4] = '\0';
+		equal = strchr(tmp, '=');
+		if (equal)
+			*equal = '\0';
+		tmp[strlen(argv[i]) - 4] = '\0';
+		if (argv[i][strlen(argv[i]) - 1] == 'd')
 			ret = snprintf(buf + used, bufsize - used,
 				       "%s%s+0x0(+0x%zx(%s)):string",
 				       equal ? tmp : "", equal ? "=" : "",
 				       offsetof(struct dentry, d_name.name),
 				       equal ? equal + 1 : tmp);
-			kfree(tmp);
-			if (ret >= bufsize - used)
-				return -ENOMEM;
-			argv[i] = buf + used;
-			used += ret + 1;
-		}
+		else
+			ret = snprintf(buf + used, bufsize - used,
+				       "%s%s+0x0(+0x%zx(+0x%zx(%s))):string",
+				       equal ? tmp : "", equal ? "=" : "",
+				       offsetof(struct dentry, d_name.name),
+				       offsetof(struct file, f_path.dentry),
+				       equal ? equal + 1 : tmp);
+		kfree(tmp);
+		if (ret >= bufsize - used)
+			return -ENOMEM;
+		argv[i] = buf + used;
+		used += ret + 1;
 	}
 
 	return 0;
-- 
2.31.1

[PATCH v2 5/7] tracing: add new type "%pd/%pD" in readme_msg[]

[Ye Bin]

Signed-off-by: Ye Bin <yebin10@huawei.com>
---
 kernel/trace/trace.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 2a7c6fd934e9..13197d3b86bd 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -5745,7 +5745,7 @@ static const char readme_msg[] =
 	"\t           +|-[u]<offset>(<fetcharg>), \\imm-value, \\\"imm-string\"\n"
 	"\t     type: s8/16/32/64, u8/16/32/64, x8/16/32/64, char, string, symbol,\n"
 	"\t           b<bit-width>@<bit-offset>/<container-size>, ustring,\n"
-	"\t           symstr, <type>\\[<array-size>\\]\n"
+	"\t           symstr, %pd/%pD <type>\\[<array-size>\\]\n"
 #ifdef CONFIG_HIST_TRIGGERS
 	"\t    field: <stype> <name>;\n"
 	"\t    stype: u8/u16/u32/u64, s8/s16/s32/s64, pid_t,\n"
-- 
2.31.1

[PATCH v2 6/7] Documentation: tracing: add new type '%pd' and '%pD' for kprobe

[Ye Bin]

Similar to printk() '%pd' is for fetch dentry's name from struct dentry's
pointer, and '%pD' is for fetch file's name from struct file's pointer.

Signed-off-by: Ye Bin <yebin10@huawei.com>
---
 Documentation/trace/kprobetrace.rst | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/Documentation/trace/kprobetrace.rst b/Documentation/trace/kprobetrace.rst
index bf9cecb69fc9..a1d12d65a8dc 100644
--- a/Documentation/trace/kprobetrace.rst
+++ b/Documentation/trace/kprobetrace.rst
@@ -58,7 +58,8 @@ Synopsis of kprobe_events
   NAME=FETCHARG : Set NAME as the argument name of FETCHARG.
   FETCHARG:TYPE : Set TYPE as the type of FETCHARG. Currently, basic types
 		  (u8/u16/u32/u64/s8/s16/s32/s64), hexadecimal types
-		  (x8/x16/x32/x64), "char", "string", "ustring", "symbol", "symstr"
+		  (x8/x16/x32/x64), VFS layer common type(%pd/%pD) for print
+                  file name, "char", "string", "ustring", "symbol", "symstr"
                   and bitfield are supported.
 
   (\*1) only for the probe on function entry (offs == 0). Note, this argument access
@@ -113,6 +114,9 @@ With 'symstr' type, you can filter the event with wildcard pattern of the
 symbols, and you don't need to solve symbol name by yourself.
 For $comm, the default type is "string"; any other type is invalid.
 
+VFS layer common type(%pd/%pD) is a special type, which fetches dentry's or
+file's name from struct dentry's pointer or struct file's pointer.
+
 .. _user_mem_access:
 
 User Memory Access
-- 
2.31.1

[PATCH v2 7/7] selftests/ftrace: add test cases for VFS type "%pd" and "%pD"

[Ye Bin]

This patch adds test cases for new print format type "%pd/%pD".The test cases
test the following items:
1. Test README if add "%pd/%pD" type;
2. Test "%pd" type for dput();
3. Test "%pD" type for vfs_read();

Signed-off-by: Ye Bin <yebin10@huawei.com>
---
 .../ftrace/test.d/kprobe/kprobe_args_vfs.tc   | 79 +++++++++++++++++++
 1 file changed, 79 insertions(+)
 create mode 100644 tools/testing/selftests/ftrace/test.d/kprobe/kprobe_args_vfs.tc

diff --git a/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_args_vfs.tc b/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_args_vfs.tc
new file mode 100644
index 000000000000..1d8edd294dd6
--- /dev/null
+++ b/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_args_vfs.tc
@@ -0,0 +1,79 @@
+#!/bin/sh
+# SPDX-License-Identifier: GPL-2.0
+# description: Kprobe event VFS type argument
+# requires: kprobe_events
+
+case `uname -m` in
+x86_64)
+  ARG1=%di
+;;
+i[3456]86)
+  ARG1=%ax
+;;
+aarch64)
+  ARG1=%x0
+;;
+arm*)
+  ARG1=%r0
+;;
+ppc64*)
+  ARG1=%r3
+;;
+ppc*)
+  ARG1=%r3
+;;
+s390*)
+  ARG1=%r2
+;;
+mips*)
+  ARG1=%r4
+;;
+loongarch*)
+  ARG1=%r4
+;;
+riscv*)
+  ARG1=%a0
+;;
+*)
+  echo "Please implement other architecture here"
+  exit_untested
+esac
+
+: "Test argument %pd/%pD in README"
+grep -q "%pd/%pD" README
+
+: "Test argument %pd with name"
+echo "p:testprobe dput name=${ARG1}:%pd" > kprobe_events
+echo 1 > events/kprobes/testprobe/enable
+grep -q "1" events/kprobes/testprobe/enable
+echo 0 > events/kprobes/testprobe/enable
+grep "dput" trace | grep -q "enable"
+echo "" > kprobe_events
+echo "" > trace
+
+: "Test argument %pd without name"
+echo "p:testprobe dput ${ARG1}:%pd" > kprobe_events
+echo 1 > events/kprobes/testprobe/enable
+grep -q "1" events/kprobes/testprobe/enable
+echo 0 > events/kprobes/testprobe/enable
+grep "dput" trace | grep -q "enable"
+echo "" > kprobe_events
+echo "" > trace
+
+: "Test argument %pD with name"
+echo "p:testprobe vfs_read name=${ARG1}:%pD" > kprobe_events
+echo 1 > events/kprobes/testprobe/enable
+grep -q "1" events/kprobes/testprobe/enable
+echo 0 > events/kprobes/testprobe/enable
+grep "vfs_read" trace | grep -q "enable"
+echo "" > kprobe_events
+echo "" > trace
+
+: "Test argument %pD without name"
+echo "p:testprobe vfs_read ${ARG1}:%pD" > kprobe_events
+echo 1 > events/kprobes/testprobe/enable
+grep -q "1"  events/kprobes/testprobe/enable
+echo 0 > events/kprobes/testprobe/enable
+grep "vfs_read" trace | grep -q "enable"
+echo "" > kprobe_events
+echo "" > trace
-- 
2.31.1

Re: [PATCH v2 1/7] string.h: add str_has_suffix() helper for test string ends with specify string

[Steven Rostedt]

On Mon, 22 Jan 2024 15:40:09 +0800
Ye Bin <yebin10@huawei.com> wrote:

> str_has_suffix() is test string if ends with specify string.
> 
> Signed-off-by: Ye Bin <yebin10@huawei.com>
> ---
>  include/linux/string.h | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/include/linux/string.h b/include/linux/string.h
> index 433c207a01da..e47e9597af27 100644
> --- a/include/linux/string.h
> +++ b/include/linux/string.h
> @@ -405,4 +405,24 @@ static __always_inline size_t str_has_prefix(const char *str, const char *prefix
>  	return strncmp(str, prefix, len) == 0 ? len : 0;
>  }
>  
> +/**
> + * str_has_suffix - Test if a string has a given suffix
> + * @str: The string to test
> + * @suffix: The string to see if @str ends with
> + *
> + * Returns:
> + * * strlen(@suffix) if @str ends with @suffix
> + * * 0 if @str does not end with @suffix
> + */
> +static __always_inline size_t str_has_suffix(const char *str, const char *suffix)
> +{
> +	size_t len = strlen(suffix);
> +	size_t str_len = strlen(str);
> +
> +	if (len > str_len)
> +		return 0;
> +
> +	return strncmp(str + str_len - len, suffix, len) == 0 ? len : 0;
> +}

I understand why you are returning the length of the suffix, as it matches
str_has_prefix(). But as the user of this will likely need the length of
the "str" too, we should either return the length of the string, or the
index of where the suffix was found.

/**
 [..]
 @index: The index into @str of where @suffix is if found (NULL to ignore)
 [..]
*/
static __always_inline size_t str_has_suffix(const char *str, const char *suffix, size_t *index)
{
	size_t len = strlen(suffix);
	size_t str_len = strlen(str);

	if (len > str_len)
		return 0;

	if (index)
		*index = str_len - len;

	return strncmp(str + str_len - len, suffix, len) == 0 ? len : 0;
}

-- Steve

> +
>  #endif /* _LINUX_STRING_H_ */

Re: [PATCH v2 2/7] tracing/probes: add traceprobe_expand_dentry_args() helper

[Steven Rostedt]

On Mon, 22 Jan 2024 15:40:10 +0800
Ye Bin <yebin10@huawei.com> wrote:

> Add traceprobe_expand_dentry_args() to expand dentry args. this API is
> prepare to support "%pd" print format for kprobe.
> 
> Signed-off-by: Ye Bin <yebin10@huawei.com>
> ---
>  kernel/trace/trace_probe.c | 34 ++++++++++++++++++++++++++++++++++
>  kernel/trace/trace_probe.h |  2 ++
>  2 files changed, 36 insertions(+)
> 
> diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c
> index 4dc74d73fc1d..1599c0c3e6b7 100644
> --- a/kernel/trace/trace_probe.c
> +++ b/kernel/trace/trace_probe.c
> @@ -1565,6 +1565,40 @@ const char **traceprobe_expand_meta_args(int argc, const char *argv[],
>  	return ERR_PTR(ret);
>  }
>  
> +int traceprobe_expand_dentry_args(int argc, const char *argv[], char *buf,
> +				  int bufsize)
> +{
> +	int i, used, ret;

	size_t idx;

> +
> +	used = 0;
> +	for (i = 0; i < argc; i++) {
> +		if (str_has_suffix(argv[i], ":%pd")) {

If we were to update str_has_suffix() as I said in my reply to the previous
patch, then we could do:

		if (str_has_suffix(argv[i], ":%pd", &idx) {

> +			char *tmp = kstrdup(argv[i], GFP_KERNEL);
> +			char *equal;
> +
> +			if (!tmp)
> +				return -ENOMEM;
> +
> +			equal = strchr(tmp, '=');
> +			if (equal)
> +				*equal = '\0';
> +			tmp[strlen(argv[i]) - 4] = '\0';

			tmp[idx] = '\0';

-- Steve

> +			ret = snprintf(buf + used, bufsize - used,
> +				       "%s%s+0x0(+0x%zx(%s)):string",
> +				       equal ? tmp : "", equal ? "=" : "",
> +				       offsetof(struct dentry, d_name.name),
> +				       equal ? equal + 1 : tmp);
> +			kfree(tmp);
> +			if (ret >= bufsize - used)
> +				return -ENOMEM;
> +			argv[i] = buf + used;
> +			used += ret + 1;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
>  void traceprobe_finish_parse(struct traceprobe_parse_context *ctx)
>  {
>  	clear_btf_context(ctx);
> diff --git a/kernel/trace/trace_probe.h b/kernel/trace/trace_probe.h
> index 850d9ecb6765..553371a4e0b1 100644
> --- a/kernel/trace/trace_probe.h
> +++ b/kernel/trace/trace_probe.h
> @@ -402,6 +402,8 @@ extern int traceprobe_parse_probe_arg(struct trace_probe *tp, int i,
>  const char **traceprobe_expand_meta_args(int argc, const char *argv[],
>  					 int *new_argc, char *buf, int bufsize,
>  					 struct traceprobe_parse_context *ctx);
> +extern int traceprobe_expand_dentry_args(int argc, const char *argv[], char *buf,
> +					 int bufsize);
>  
>  extern int traceprobe_update_arg(struct probe_arg *arg);
>  extern void traceprobe_free_probe_arg(struct probe_arg *arg);

Re: [PATCH v2 4/7] tracing/probes: support '%pD' type for print struct file's name

[Steven Rostedt]

On Mon, 22 Jan 2024 15:40:12 +0800
Ye Bin <yebin10@huawei.com> wrote:

> Similar to '%pD' for printk, use '%pD' for print struct file's name.
> 
> Signed-off-by: Ye Bin <yebin10@huawei.com>
> ---
>  kernel/trace/trace_probe.c | 41 ++++++++++++++++++++++++--------------
>  1 file changed, 26 insertions(+), 15 deletions(-)
> 
> diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c
> index 1599c0c3e6b7..f9819625de58 100644
> --- a/kernel/trace/trace_probe.c
> +++ b/kernel/trace/trace_probe.c
> @@ -12,6 +12,7 @@
>  #define pr_fmt(fmt)	"trace_probe: " fmt
>  
>  #include <linux/bpf.h>
> +#include <linux/fs.h>
>  #include "trace_btf.h"
>  
>  #include "trace_probe.h"
> @@ -1572,28 +1573,38 @@ int traceprobe_expand_dentry_args(int argc, const char *argv[], char *buf,
>  
>  	used = 0;
>  	for (i = 0; i < argc; i++) {
> -		if (str_has_suffix(argv[i], ":%pd")) {
> -			char *tmp = kstrdup(argv[i], GFP_KERNEL);
> -			char *equal;
> +		if (!str_has_suffix(argv[i], ":%pd") &&
> +		    !str_has_suffix(argv[i], ":%pD"))
> +			continue;

And here too:

		if (!str_has_suffix(argv[i], ":%pd", &idx) &&
		    !str_has_suffix(argv[i], ":%pD", &idx))
			continue;


>  
> -			if (!tmp)
> -				return -ENOMEM;
> +		char *tmp = kstrdup(argv[i], GFP_KERNEL);
> +		char *equal;
> +
> +		if (!tmp)
> +			return -ENOMEM;
>  
> -			equal = strchr(tmp, '=');
> -			if (equal)
> -				*equal = '\0';
> -			tmp[strlen(argv[i]) - 4] = '\0';
> +		equal = strchr(tmp, '=');
> +		if (equal)
> +			*equal = '\0';
> +		tmp[strlen(argv[i]) - 4] = '\0';

		tmp[idx] = '\0';

> +		if (argv[i][strlen(argv[i]) - 1] == 'd')

To avoid another strlen() call.

		if (tmp[idx + 3] == 'd')


-- Steve


>  			ret = snprintf(buf + used, bufsize - used,
>  				       "%s%s+0x0(+0x%zx(%s)):string",
>  				       equal ? tmp : "", equal ? "=" : "",
>  				       offsetof(struct dentry, d_name.name),
>  				       equal ? equal + 1 : tmp);
> -			kfree(tmp);
> -			if (ret >= bufsize - used)
> -				return -ENOMEM;
> -			argv[i] = buf + used;
> -			used += ret + 1;
> -		}
> +		else
> +			ret = snprintf(buf + used, bufsize - used,
> +				       "%s%s+0x0(+0x%zx(+0x%zx(%s))):string",
> +				       equal ? tmp : "", equal ? "=" : "",
> +				       offsetof(struct dentry, d_name.name),
> +				       offsetof(struct file, f_path.dentry),
> +				       equal ? equal + 1 : tmp);
> +		kfree(tmp);
> +		if (ret >= bufsize - used)
> +			return -ENOMEM;
> +		argv[i] = buf + used;
> +		used += ret + 1;
>  	}
>  
>  	return 0;

Re: [PATCH v2 1/7] string.h: add str_has_suffix() helper for test string ends with specify string

[yebin (H)]

On 2024/1/23 0:07, Steven Rostedt wrote:
> On Mon, 22 Jan 2024 15:40:09 +0800
> Ye Bin <yebin10@huawei.com> wrote:
>
>> str_has_suffix() is test string if ends with specify string.
>>
>> Signed-off-by: Ye Bin <yebin10@huawei.com>
>> ---
>>   include/linux/string.h | 20 ++++++++++++++++++++
>>   1 file changed, 20 insertions(+)
>>
>> diff --git a/include/linux/string.h b/include/linux/string.h
>> index 433c207a01da..e47e9597af27 100644
>> --- a/include/linux/string.h
>> +++ b/include/linux/string.h
>> @@ -405,4 +405,24 @@ static __always_inline size_t str_has_prefix(const char *str, const char *prefix
>>   	return strncmp(str, prefix, len) == 0 ? len : 0;
>>   }
>>   
>> +/**
>> + * str_has_suffix - Test if a string has a given suffix
>> + * @str: The string to test
>> + * @suffix: The string to see if @str ends with
>> + *
>> + * Returns:
>> + * * strlen(@suffix) if @str ends with @suffix
>> + * * 0 if @str does not end with @suffix
>> + */
>> +static __always_inline size_t str_has_suffix(const char *str, const char *suffix)
>> +{
>> +	size_t len = strlen(suffix);
>> +	size_t str_len = strlen(str);
>> +
>> +	if (len > str_len)
>> +		return 0;
>> +
>> +	return strncmp(str + str_len - len, suffix, len) == 0 ? len : 0;
>> +}
> I understand why you are returning the length of the suffix, as it matches
> str_has_prefix(). But as the user of this will likely need the length of
> the "str" too, we should either return the length of the string, or the
> index of where the suffix was found.
>
> /**
>   [..]
>   @index: The index into @str of where @suffix is if found (NULL to ignore)
>   [..]
> */
> static __always_inline size_t str_has_suffix(const char *str, const char *suffix, size_t *index)
> {
> 	size_t len = strlen(suffix);
> 	size_t str_len = strlen(str);
>
> 	if (len > str_len)
> 		return 0;
>
> 	if (index)
> 		*index = str_len - len;
>
> 	return strncmp(str + str_len - len, suffix, len) == 0 ? len : 0;
> }
>
> -- Steve
Thank you for your advice.
Return the index of where the suffix was found is useful. I'll modify it 
according to your suggestion.

-- Ye Bin
>> +
>>   #endif /* _LINUX_STRING_H_ */
> .
>