Re: [PATCH 04/14] x86/fpu: Remove 'kbuf' parameter from the copy_xstate_to_user() APIs
From: Ingo Molnar
Date: Mon Jan 30 2017 - 05:00:56 EST
* Borislav Petkov <bp@xxxxxxxxx> wrote:
> On Thu, Jan 26, 2017 at 11:22:49AM +0100, Ingo Molnar wrote:
> > The 'kbuf' parameter is unused in the _user() side of the API, remove it.
> > This simplifies the code and makes it easier to think about.
> > @@ -1010,10 +1010,7 @@ int copy_xstate_to_kernel(unsigned int pos, unsigned int count, void *kbuf, stru
> > }
> > static inline int
> > -__copy_xstate_to_user(unsigned int pos, unsigned int count,
> > - void *kbuf, void __user *ubuf,
> > - const void *data, const int start_pos,
> > - const int end_pos)
> > +__copy_xstate_to_user(unsigned int pos, unsigned int count, void __user *ubuf, const void *data, const int start_pos, const int end_pos)
> That and similar lines are insanely long and could be broken.
Yeah, so that's one point of the series: the interface was insanely complex (the
original sin is that of the regset interfaces), and one symptom of that complexity
are these overly long prototypes - the above one has 7 arguments (!!). Another,
far more serious symptom of the complexity were the bugs that Rik found.
The solution was not to break the prototype into multiple lines and thus paper
over one symptom of complexity, but to _reduce_ complexity.
So at the end of the series the basic copy_xstate_to_user() prototype looks like
static inline int
__copy_xstate_to_user(void __user *ubuf, const void *data, unsigned int offset, unsigned int size, unsigned int size_total)
which is less complex and shorter as well. It could probably be shortened further:
static inline int
__copy_xstate_to_user(void __user *ubuf, const void *data, u32 offset, u32 size, u32 total)
because our regset (and user-copy) APIs are intentionally 32-bit - but this would
depart from the existing signature style so I'm not sure we want to do it
Would anyone object to using u32 in these prototypes?