Messages in this thread | | | Date | Mon, 13 Mar 2023 12:03:35 +0100 | From | Borislav Petkov <> | Subject | Re: [PATCH v7 38/41] x86/fpu: Add helper for initing features |
| |
On Mon, Mar 13, 2023 at 02:45:08AM +0000, Edgecombe, Rick P wrote: > These two are from the existing code. Basically they get extracted into > a new function.
I know but you can fix them while at it.
> I did it up, and it makes the caller code cleaner. But I'm not sure > what to think of it. Is this not mixing two operations together? Today > get_xsave_addr() pretty much just gets a buffer offset with some > checks. Now it would compute the offset and also silently go off and > changes the buffer.
Ok, so why don't you write the call site this way instead:
cetregs = get_xsave_addr(xsave, XFEATURE_CET_USER); if (!cetregs) { if (xfeature_saved(xsave, XFEATURE_CET_USER)) { WARN("something's wrong with this buffer") return ...; }
/* Not saved, initialize it */ init_xfeature(xsave, XFEATURE_CET_USER)); }
cetregs = get_xsave_addr(xsave, XFEATURE_CET_USER); if (!cetregs) { WARN_ON("WTF") return -ENODEV; }
Now it is clear what happens and it is a common code pattern of trying to get something and initializing it if it wasn't initialized yet, and then retrying...
Hmm?
-- Regards/Gruss, Boris.
https://people.kernel.org/tglx/notes-about-netiquette
| |