Re: [PATCH 1/5] Split wait_noreap_copyout()

Ingo Molnar
2009-05-11T10:21:55Z

* Vitaly Mayatskikh <v.mayatskih@gmail.com> wrote:

> -static int wait_noreap_copyout(struct wait_opts *wo, struct task_struct *p,
> - pid_t pid, uid_t uid, int why, int status)
> +static int wait_copyout(struct wait_opts *wo, struct task_struct *p,
> + pid_t pid, uid_t uid, int why, int status, int signal)

Nice cleanups. Would be nice to fix the naming here too while at it.

Right now it's two verbs and a straightforward reading of it suggest
that we 'wait for some copyout to occur', which is nonsensical and
confusing.

So please put the main action as the first verb (this is an internal
symbol so no subsystem differentiator is needed). Something like:

copy_wait_opts_to_user()

... and it becomes a whole lot easier to read. This matches the
copy*to_user idioms we have elsewhere so it nicely wibes with the
sound of those.

( Now repeat similar measures on the whole tree, a hundred thousand
times or so, and enforce it for all new patches, and we'd have
crisp, consistent, highly readable and enjoyable kernel source ;-)

Thanks,

Ingo