Re: [PATCH] firmware: dmi_scan: Fix ordering of product_uuid

[Zhenzhong Duan]

å 2015/4/16 17:30, Jean Delvare åé:
> Le Thursday 16 April 2015 Ã 16:46 +0800, Zhenzhong Duan a Ãcrit :
> > On 2015/4/16 15:09, Jean Delvare wrote:
> > > Le Thursday 16 April 2015 Ã 14:22 +0800, Zhenzhong Duan a Ãcrit :
> > > > The basic idea is right, but you ignore the case dmi_walk_early may
> > > > fail, though looks impossible when bootup.
> > > >
> > > > Better to add below for robust.
> > > >
> > > > @@ -521,6 +521,6 @@ static int __init dmi_present(const u8 *
> > > >
> > > >                 return 0;
> > > >             }
> > > >         }
> > > > +    dmi_ver = 0;
> > > >         return 1;
> > > >     }
> > > What is the value of this? dmi_ver will never be accessed after this
> > > point anyway, as far as I can see.
> > Same as above, future commit may not realize you bring this faulty when
> > they want to use dmi_ver.
> Why do you think this is "faulty"? The value in dmi_ver is correct
> whether dmi_walk_early() succeeded or not. There's no rationale for
> resetting dmi_ver on error and not dmi_num, dmi_len and dmi_base. Note
> that dmi_smbios3_present() doesn't reset any of these either. These
> values are all correct.
>
> If other modules need to check whether DMI was successfully initialized,
> they must check dmi_available rather than any of the variables above
> (which are all static anyway.)

You are right, dmi_available should be used here. Sorry for noise

zduan

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/