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
