On Tue, 7 Jan 2025 10:36:43 -0500
Steven Rostedt <rost...@goodmis.org> wrote:
> On Tue, 7 Jan 2025 20:50:25 +0900
> "Masami Hiramatsu (Google)" <mhira...@kernel.org> wrote:
>
> > @@ -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
>
>
> > 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) <mhira...@kernel.org>
