On Mon, Jun 29, 2020 at 12:23:34PM -0700, Linus Torvalds wrote:
> On Mon, Jun 29, 2020 at 11:28 AM Al Viro <viro@xxxxxxxxxxxxxxxxxx> wrote:
> >
> > ->get2() takes task+regset+buffer, returns the amount of free space
> > left in the buffer on success and -E... on error.
>
> Can we please give it a better name than "get2"?
>
> That's not a great name to begin with, and it's a completely
> nonsensical name by the end of this series when you've removed the
> original "get" function.
>
> So either:
>
> (a) add one final patch to rename "get2" all back to "get" after you
> got rid of the old "get"
Bad idea, IMO...
> (b) or just call it something better to begin with. Maybe just
> "get_regset" instead?
<nod>
Frankly, other field names there are also nasty - 'set' is not
fun to grep for, but there are 'n' and 'size' as well. There's
also 'bias' and 'align' (both completely unused)...
> I'd prefer (b) just because I think it will be a lot clearer if we
> ever end up having old patches forward-ported (or, more likely,
> newpatches back-ported), so having a "get" function that changed
> semantics but got back the old name sounds bad to me.
>
> Other than that, I can't really argue with the numbers:
>
> 41 files changed, 1368 insertions(+), 2347 deletions(-)
>
> looks good to me, and the code seems more understandable.
FWIW, there's also ->set() side of that thing. Tons of boilerplate
in those; I hadn't seriously looked into that part, but I suspect
that "do copyin in the caller, pass the kernel buffer to the
method, always start at offset 0" would also trim a lot of crap.
I'd rather leave that one for later - this series had been painful
enough.
Other things in the same area: conversion of fdpic coredumps to
regset (fairly easy, I've a patch series doing that in the local
tree), conversion of the few remaining non-regset architectures
to regset-based coredump (kill a large chunk of rotting code in
fs/binfmt_elf.c, along with quite a few pieces of old per-arch
coredump helpers), unification of compat ELF (done locally,
mipsn32/mipso32 gone, all compat done via compat_binfmt_elf.c,
x32 horrors sanitized - IMO
#define PRSTATUS_SIZE \
(test_thread_flag(TIF_X32) \
? sizeof(struct compat_elf_prstatus) \
: sizeof(struct i386_elf_prstatus))
#define SET_PR_FPVALID(S) \
(*(test_thread_flag(TIF_X32) \
? &(S)->pr_fpvalid \
: &((struct i386_elf_prstatus *)(S))->pr_fpvalid) = 1)
is much better than
#define PRSTATUS_SIZE(S, R) (R != sizeof(S.pr_reg) ? 144 : 296)
#define SET_PR_FPVALID(S, V, R) \
do { *(int *) (((void *) &((S)->pr_reg)) + R) = (V); } \
while (0)
) and a bunch of assorted cleanups that got trimmed from the
regset series...
I'll probably post fdpic and compat series tonight and get
back to uaccess and VFS stuff.
