From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=209.132.183.28; helo=mx1.redhat.com; envelope-from=lersek@redhat.com; receiver=edk2-devel@lists.01.org Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 909E821A09130 for ; Wed, 10 Oct 2018 00:21:18 -0700 (PDT) Received: from smtp.corp.redhat.com (int-mx07.intmail.prod.int.phx2.redhat.com [10.5.11.22]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 3F7E4B2CD; Wed, 10 Oct 2018 07:21:18 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-120-122.rdu2.redhat.com [10.10.120.122]) by smtp.corp.redhat.com (Postfix) with ESMTP id 4359B10018FF; Wed, 10 Oct 2018 07:21:17 +0000 (UTC) To: Dongao Guo , edk2-devel@lists.01.org Cc: Liming Gao References: <1539139366-2708-1-git-send-email-dongao.guo@intel.com> From: Laszlo Ersek Message-ID: Date: Wed, 10 Oct 2018 09:21:16 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: <1539139366-2708-1-git-send-email-dongao.guo@intel.com> X-Scanned-By: MIMEDefang 2.84 on 10.5.11.22 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.26]); Wed, 10 Oct 2018 07:21:18 +0000 (UTC) Subject: Re: [PATCH v4] MdeModulePkg/RegularExpressionDxe:disable wraning to pass gcc4.8 build X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 10 Oct 2018 07:21:19 -0000 Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Hi Dongao, On 10/10/18 04:42, Dongao Guo wrote: > There are three warnings reported by GCC 4.8 and the later GCC release > are workaround with them. > And all the three warnings are invalid,so I just disable warnings rather > than fix them at now. > > Following is the analysis from Laszlo Ersek. > (1) > >> MdeModulePkg/Universal/RegularExpressionDxe/Oniguruma/regcomp.c: In >> function 'compile_length_tree': >> MdeModulePkg/Universal/RegularExpressionDxe/Oniguruma/regcomp.c:1516:7: >> warning: 'len' may be used uninitialized in this function >> [-Wmaybe-uninitialized] >> int len; >> ^ > > I think this is an invalid warning; the type of the controlling expression > (node->type) is enum GimmickType, and the case labels cover all values of > the enum. An assert(0) could be added, I guess, but again, upstream > Oniguruma would be justified to reject the idea. > > (2) > >> MdeModulePkg/Universal/RegularExpressionDxe/Oniguruma/regparse.c: In >> function 'parse_callout_args.isra.10.constprop.30': >> MdeModulePkg/Universal/RegularExpressionDxe/Oniguruma/regparse.c:6753:25: >> warning: 'rl' may be used uninitialized in this function >> [-Wmaybe-uninitialized] >> vals[n].l = rl; >> ^ > > This warning is invalid, given: > > 6749 if (cn > 0) { > 6750 long rl; > 6751 r = parse_long(enc, buf, bufend, 1, LONG_MAX, &rl); > 6752 if (r == ONIG_NORMAL) { > 6753 vals[n].l = rl; > > Because parse_long() only returns ONIG_NORMAL after it sets (*rl). > > (3) > >> MdeModulePkg/Universal/RegularExpressionDxe/Oniguruma/regparse.c: In >> function 'parse_callout_of_name.constprop.29': >> MdeModulePkg/Universal/RegularExpressionDxe/Oniguruma/regparse.c:6861:38: >> warning: 'tag_end' may be used uninitialized in this function >> [-Wmaybe-uninitialized] >> if (! is_allowed_callout_tag_name(enc, tag_start, tag_end)) > > This warning is also invalid, given: > > 6852 if (c == '[') { > 6853 if (PEND) return ONIGERR_END_PATTERN_IN_GROUP; > 6854 tag_start = p; > 6855 while (! PEND) { > 6856 if (PEND) return ONIGERR_END_PATTERN_IN_GROUP; > 6857 tag_end = p; > 6858 PFETCH_S(c); > 6859 if (c == ']') break; > 6860 } > 6861 if (! is_allowed_callout_tag_name(enc, tag_start, tag_end)) > 6862 return ONIGERR_INVALID_CALLOUT_TAG_NAME; > 6863 > > To see that, first we should note: (1) Here, my original email at , contained another line: #define PEND (p < end ? 0 : 1) This line is not part of your commit message, because "git commit" considers all lines that start with "#" a comment, and they are removed from the final commit message. When quoting content that may contain such lines, it is usually best to either indent the quote by a few space characters, or else prefix each line with the string "> ", or something similar. > > therefore PEND doesn't change if neither "p" nor "end" change. > > Second, when we reach line 6855 (the "while") for the very first time, > (!PEND) is certainly true (i.e., PEND is false), because otherwise we > would have bailed at line 6853. Therefore we reach line 6857, and assign > "tag_end". Regardless of whether we iterate zero or more *additional* > times around the loop, "tag_end" will have been set, whenever we reach > line 6861. > > Cc: Liming Gao > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Dongao Guo > --- > MdeModulePkg/Universal/RegularExpressionDxe/RegularExpressionDxe.inf | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/MdeModulePkg/Universal/RegularExpressionDxe/RegularExpressionDxe.inf b/MdeModulePkg/Universal/RegularExpressionDxe/RegularExpressionDxe.inf > index 16e91bd..07bc02e 100644 > --- a/MdeModulePkg/Universal/RegularExpressionDxe/RegularExpressionDxe.inf > +++ b/MdeModulePkg/Universal/RegularExpressionDxe/RegularExpressionDxe.inf > @@ -109,3 +109,6 @@ > > # Oniguruma: error: variable 'fp' set but not used > GCC:*_*_*_CC_FLAGS = -Wno-error=unused-but-set-variable > + > + # Oniguruma: tag_end in parse_callout_of_name (2) Based on the analysis now included in the commit message, I suggest: - either dropping this comment (because it is incomplete), - or including all three cases briefly. > + GCC:*_*_*_CC_FLAGS = -Wno-error=maybe-uninitialized > In my opinion, (1) should be fixed, because otherwise the commit message is confusing. Regarding (2), I don't feel strongly, so if you disagree, I don't mind. Thanks! Laszlo