[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] regexec: fix unintentinal fallthrough warning with GCC7
From: |
Andrei Borzenkov |
Subject: |
Re: [PATCH] regexec: fix unintentinal fallthrough warning with GCC7 |
Date: |
Thu, 23 Mar 2017 19:27:24 +0300 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.7.0 |
23.03.2017 19:21, Eric Blake пишет:
> On 03/23/2017 11:16 AM, Andrei Borzenkov wrote:
>> [ 104s] In file included from ../../grub-core/gnulib/regex.c:72:0:
>> [ 104s] ../../grub-core/gnulib/regexec.c: In function 'check_node_accept':
>> [ 104s] ../../grub-core/gnulib/regexec.c:4100:10: error: this statement may
>> fall through [-Werror=implicit-fallthrough=]
>> [ 104s] if (ch >= ASCII_CHARS)
>> [ 104s] ^
>> [ 104s] ../../grub-core/gnulib/regexec.c:4104:5: note: here
>> [ 104s] case OP_PERIOD:
>> [ 104s] ^~~~
>>
>> The code in question does have FALLTHROUGH annotation; unfortunately GCC7
>> ignores it, because it is separated from case statement by #endif.
>>
>> The patch fixes it by using new attribute that is honored even inside
>> #ifdef'ed
>> code.
>>
>> ---
>> lib/regexec.c | 3 +++
>> 1 file changed, 3 insertions(+)
>>
>> diff --git a/lib/regexec.c b/lib/regexec.c
>> index ef52b24..4588e13 100644
>> --- a/lib/regexec.c
>> +++ b/lib/regexec.c
>> @@ -4078,6 +4078,9 @@ check_node_accept (const re_match_context_t *mctx,
>> const re_token_t *node,
>> case OP_UTF8_PERIOD:
>> if (ch >= ASCII_CHARS)
>> return false;
>> +#if defined __GNUC__ && __GNUC__ >= 7
>> + __attribute__ ((fallthrough));
>> +#endif
>> /* FALLTHROUGH */
>> #endif
>> case OP_PERIOD:
>
>
> Would it also work if you did:
>
> case OP_UTF8_PERIOD:
> if (ch >= ASCII_CHARS)
> return false;
> #endif
> /* FALLTHROUGH */
> case OP_PERIOD:
>
> (that is, swap the #endif and FALLTHROUGH lines)?
>
The result is wrong when code is ifdef'ed out, because preceding case
label must *not* fall through into this one.
case SIMPLE_BRACKET:
if (!bitset_contain (node->opr.sbcset, ch))
return false;
break;
#ifdef RE_ENABLE_I18N
case OP_UTF8_PERIOD:
if (ch >= ASCII_CHARS)
return false;
/* FALLTHROUGH */
#endif
case OP_PERIOD:
If RE_ENABLE_I18N is not defined, FALLTHROUGH will be attributed to
wrong branch. If someone happens to add case label before #ifdef, it
will be rather hard to spot.
I thought about
#ifdef RE_ENABLE_I18N
case OP_UTF8_PERIOD:
if (ch >= ASCII_CHARS)
return false;
/* FALLTHROUGH */
case OP_PERIOD:
#else
case OP_PERIOD:
#endif
But it just looks ugly (and I did not test it).
signature.asc
Description: OpenPGP digital signature