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
<e4131806-14d5-b11b-5b68-2c552fcadf0c@redhat.com">http://mid.mail-archive.com/e4131806-14d5-b11b-5b68-2c552fcadf0c@redhat.com>,
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 <liming....@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Dongao Guo <dongao....@intel.com>
> ---
> 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
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
