Re: [PATCH v3 2/5] tracing: Use __free() in trace_probe for cleanup

On Tue, 7 Jan 2025 10:36:43 -0500
Steven Rostedt [off-list ref] wrote:

On Tue,  7 Jan 2025 20:50:25 +0900
"Masami Hiramatsu (Google)" [off-list ref] wrote:

quoted

@@ -1790,18 +1777,15 @@ int traceprobe_expand_dentry_args(int argc, const char *argv[], char **buf)
 				       offsetof(struct file, f_path.dentry),
 				       equal ? equal + 1 : tmp);
 
-		kfree(tmp);
+		kfree(no_free_ptr(tmp));

I don't get this? You are telling the compiler not to free tmp, because you
decided to free it yourself? Why not just remove the kfree() here altogether?

In the for-loop block, the __free() work only when we exit the loop, not
each iteration. In each iteration, kstrdup() is assigned to the 'tmp',
so we need to kfree() each time.

Hmm, maybe this is a sign that I should not use __free() for the 'tmp',
or I should call kfree(tmp) right before kstrdup(), like below.

 	for (i = 0; i < argc; i++) {
		char *tmp __free(kfree) = NULL;
		...
		kfree(tmp);
		tmp = kstrdup(argv[i], GFP_KERNEL);
	}

Does this make sense?

-- Steve

quoted

 		if (ret >= bufsize - used)
-			goto nomem;
+			return -ENOMEM;
 		argv[i] = tmpbuf + used;
 		used += ret + 1;
 	}
 
-	*buf = tmpbuf;
+	*buf = no_free_ptr(tmpbuf);
 	return 0;
-nomem:
-	kfree(tmpbuf);
-	return -ENOMEM;
 }

-- 
Masami Hiramatsu (Google) [off-list ref]