Re: [PATCH v2 2/3] x86/ucode: Fix cache handling in microcode_update_helper()

[Jan Beulich <jbeulich@xxxxxxxx>]

On 15.11.2024 19:03, Andrew Cooper wrote:
> On 14/11/2024 11:26 am, Jan Beulich wrote:
>> On 12.11.2024 22:19, Andrew Cooper wrote:
>>> microcode_update_cache() now has a single caller, but inlining it shows how
>>> unnecessarily complicated the logic really is.
>>>
>>> Outside of error paths, there is always one microcode patch to free.  Its
>>> either result of parse_blob(), or it's the old cached value.
>>>
>>> In order to fix this, have a local patch pointer (mostly to avoid the
>>> unnecessary verbosity of patch_with_flags.patch), and always free it at the
>>> end.  The only error path needing care is the IS_ERR(patch) path, which is
>>> easy enough to handle.
>>>
>>> Also, widen the scope of result.  We only need to call compare_patch() once,
>>> and the answer is still good later when updating the cache.  In order to
>>> update the cache, simply SWAP() the patch and the cache pointers, allowing 
>>> the
>>> singular xfree() at the end to cover both cases.
>>>
>>> This also removes all callers microcode_free_patch() which fixes the need to
>>> cast away const to allow it to compile.
>> I'm sure you're well aware that this in turn is just because of your 
>> opposition
>> to xfree() and alike taking const void *.
> 
> My opposition, and the C standards committee, and MISRA to name but a few.

The C standard's committee - yes, if we mean to stick to free()'s properties
to the letter (I don't think they have any idea about unmap-like things). For
Misra though, would you mind educating me where they would have a concern?

Jan