On 14.11.2024 18:18, Andrew Cooper wrote:
> On 14/11/2024 11:41 am, Jan Beulich wrote:
>> On 12.11.2024 22:19, Andrew Cooper wrote:
>>> @@ -199,8 +198,8 @@ static bool microcode_fits_cpu(const struct
>>> microcode_patch *patch)
>>> return equiv.id == patch->processor_rev_id;
>>> }
>>>
>>> -static enum microcode_match_result cf_check compare_patch(
>>> - const struct microcode_patch *new, const struct microcode_patch *old)
>>> +static int cf_check compare_patch(
>>> + const struct microcode_patch *old, const struct microcode_patch *new)
>>> {
>> Let's hope we won't screw up a backport because of this swapping.
>
> I wasn't going to start thinking about backports until the code gets
> into a better state.
>
> But if backports do happen, it will be all-or-nothing. This code is far
> too tangled.
I wasn't so much worrying about backporting of this work (as of now I don't
think it's a candidate), but anything that's yet to come.
> That said, in this specific case, the only thing that would go wrong is
> with Intel debug patches. Even I've only had a handful of those in the
> past 8 years.
Why would that be? Doing the check the wrong way round would lead to
possible downgrading of ucode, wouldn't it?
>> I'd like to ask to at least consider renaming at least the functions,
>> perhaps also the hook pointer, perhaps simply by switching from singular
>> to plural. This would then also avoid reviewers like me to go hunt for
>> all uses of the function/hook, in an attempt to make sure none was left
>> out when converting.
>
> In the other series I've paused for a while, I have renamed some hooks
> (along with related cleanup), but I'm undecided on this one.
>
> One option is cmp(), or perhaps compare().
Either would be fine with me as a hook name. As a function name I'm less
certain this will (remain to) be unambiguous.
> But, it occurs to me, another option would be is_newer(). We always
> care about the operation one way around.
is_newer() doesn't very well lend itself to a tristate return value.
Jan
