Messages in this thread | | | Date | Fri, 10 Apr 2015 13:54:56 +0200 | From | Quentin Casasnovas <> | Subject | Re: [PATCH 07/13] x86/microcode/intel: Rename update_match_revision() |
| |
On Fri, Apr 10, 2015 at 01:12:18PM +0200, Borislav Petkov wrote: > On Tue, Feb 24, 2015 at 05:23:18PM +0100, Quentin Casasnovas wrote: > > Minor nit-pick, if you reverse your inequality, you don't need for the > > ternary operator. > > Yeah, so I started looking at that and it seems the rabbit hole goes > deeper. > > Let's look at the call to revision_is_newer() in _save_mc(): > > save_mc: > > new_rev = mc_hdr->rev; > > ... > > if (!revision_is_newer(mc_hdr, new_rev)) > ->
Ha good catch!
BTW, I could not find that the 'rev' argument to get_matching_sig() was used at all..
> > --- > From: Borislav Petkov <bp@suse.de> > Date: Fri, 10 Apr 2015 12:50:57 +0200 > Subject: [PATCH] x86/microcode/intel: Get rid of revision_is_newer() > > ... > > diff --git a/arch/x86/kernel/cpu/microcode/intel_lib.c b/arch/x86/kernel/cpu/microcode/intel_lib.c > index cd47a510a3f1..63b0a2e059ee 100644 > --- a/arch/x86/kernel/cpu/microcode/intel_lib.c > +++ b/arch/x86/kernel/cpu/microcode/intel_lib.c > @@ -154,13 +154,13 @@ int get_matching_sig(unsigned int csig, int cpf, int rev, void *mc) > /* > * Returns 1 if update has been found, 0 otherwise. > */ > -int get_matching_microcode(unsigned int csig, int cpf, int rev, void *mc) > +int get_matching_microcode(unsigned int csig, int cpf, int new_rev, void *mc)
If we're to rename 'rev', maybe calling it 'cpu_rev' would make it more obvious where this variable is coming from?
> { > struct microcode_header_intel *mc_hdr = mc; > > - if (!revision_is_newer(mc_hdr, rev)) > + if (mc_hdr->rev <= new_rev) > return 0; > > - return get_matching_sig(csig, cpf, rev, mc); > + return get_matching_sig(csig, cpf, new_rev, mc); > } > EXPORT_SYMBOL_GPL(get_matching_microcode);
Anyway you patch looks good to me!
Quentin
| |