From: Zhenzhong Duan <zhenzhong.duan@oracle.com>
To: Jean Delvare <jdelvare@suse.de>
Cc: LKML <linux-kernel@vger.kernel.org>,
Ben Hutchings <ben@decadent.org.uk>,
Artem Savkov <artem.savkov@gmail.com>,
Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>,
Matt Fleming <matt.fleming@intel.com>
Subject: Re: [PATCH] firmware: dmi_scan: Fix ordering of product_uuid
Date: Fri, 17 Apr 2015 09:04:49 +0800 [thread overview]
Message-ID: <55305C31.4060104@oracle.com> (raw)
In-Reply-To: <1429176638.4386.19.camel@chaos.site>
在 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
prev parent reply other threads:[~2015-04-17 1:04 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-04-15 9:02 [PATCH] firmware: dmi_scan: Fix ordering of product_uuid Jean Delvare
2015-04-16 6:22 ` Zhenzhong Duan
2015-04-16 7:09 ` Jean Delvare
2015-04-16 8:46 ` Zhenzhong Duan
2015-04-16 9:30 ` Jean Delvare
2015-04-17 1:04 ` Zhenzhong Duan [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=55305C31.4060104@oracle.com \
--to=zhenzhong.duan@oracle.com \
--cc=artem.savkov@gmail.com \
--cc=ben@decadent.org.uk \
--cc=ivan.khoronzhuk@linaro.org \
--cc=jdelvare@suse.de \
--cc=linux-kernel@vger.kernel.org \
--cc=matt.fleming@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.