Re: [PATCH] regexec: fix unintentinal fallthrough warning with GCC7

[Eric Blake]

On 03/23/2017 11:27 AM, Andrei Borzenkov wrote:

>>
>> (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.

Rather than "must not", it "does not" fall through.  But does gcc warn
if the magic /* FALLTHROUGH */ comment is present even when it is not
utilized?

> 
>     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.

Yes, but that should be harmless.

> If someone happens to add case label before #ifdef, it
> will be rather hard to spot.

Isn't that what code review is for?

> 
> 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).

Indeed too ugly to consider.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

signature.asc
Description: OpenPGP digital signature