Messages in this thread | | | Date | Fri, 15 Sep 2023 23:08:51 +0200 | From | Peter Zijlstra <> | Subject | Re: Buggy __free(kfree) usage pattern already in tree |
| |
On Fri, Sep 15, 2023 at 01:40:25PM -0700, Linus Torvalds wrote:
> Not because I think it's necessarily any kind of final rule, but > because I think our whole cleanup thing is new enough that I think > we're better off being a bit inflexible, and having a syntax where a > simple "grep" ends up showing pretty much exactly what is going on wrt > the pairing.
So in the perf-event conversion patches I do have this:
struct task_struct *task __free(put_task) = NULL;
...
if (pid != -1) { task = find_lively_task_by_vpid(pid); if (!task) return -ESRCH; }
...
pattern. The having of task is fully optional in the code-flow.
I suppose I can try and rewrite that a little something like:
...
struct task_struct *task __free(put_task) = find_lively_task_by_vpid(pid); /* ensure pid==-1 returns NULL */
if (!task && pid > 0) return -ESRCH;
...
But a little later in that same function I then have:
do { struct rw_semaphore *exec_update_lock __free(up_read) = NULL; if (task) { err = down_read_interruptible(&task->signal->exec_update_lock); if (err) return err;
exec_update_lock = &task->signal->exec_update_lock;
if (!perf_check_permissions(&attr, task)) return -EACCESS; }
... stuff serialized against exec *if* this is a task event ...
} while (0);
And that might be a little harder to 'fix'.
I suppose I'm saying that when thing truly are conditional, this is a useful pattern, but avoid where reasonably possible.
| |