Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 11 Jan 2021 23:24:13 +0300
From:      Vladimir Kondratyev <vladimir@kondratyev.su>
To:        Ryan Libby <rlibby@freebsd.org>, Jessica Clarke <jrtc27@freebsd.org>
Cc:        John Baldwin <jhb@freebsd.org>, "src-committers@freebsd.org" <src-committers@freebsd.org>, "dev-commits-src-all@freebsd.org" <dev-commits-src-all@freebsd.org>, "dev-commits-src-main@freebsd.org" <dev-commits-src-main@freebsd.org>
Subject:   Re: git: 16079c7233be - main - hid: quiet -Wswitch
Message-ID:  <e42a1337-d268-31b0-7c68-a677435491b2@kondratyev.su>
In-Reply-To: <CAHgpiFwQsxkEVqD_-Qjwy3SyCz0q3YrKNjYScrAewA821peUSw@mail.gmail.com>
References:  <202101110554.10B5sW2q070743@gitrepo.freebsd.org> <700dd42d-2d73-e54a-5fcc-b62ed31df80d@FreeBSD.org> <915475fa-0072-2303-dfc9-dbeb42224434@kondratyev.su> <486B154B-376A-4000-8946-844353D8504E@freebsd.org> <CAHgpiFwQsxkEVqD_-Qjwy3SyCz0q3YrKNjYScrAewA821peUSw@mail.gmail.com>

next in thread | previous in thread | raw e-mail | index | archive | help
On 11.01.2021 22:36, Ryan Libby wrote:
> On Mon, Jan 11, 2021 at 11:22 AM Jessica Clarke <jrtc27@freebsd.org> wrote:
>>
>> On 11 Jan 2021, at 19:14, Vladimir Kondratyev <vladimir@kondratyev.su> wrote:
>>> On 11.01.2021 21:03, John Baldwin wrote:
>>>> On 1/10/21 9:54 PM, Ryan Libby wrote:
>>>>> The branch main has been updated by rlibby:
>>>>>
>>>>> URL: https://cgit.FreeBSD.org/src/commit/?id=16079c7233be8bd6c88e3421a70c7ca87cfea370
>>>>>
>>>>> commit 16079c7233be8bd6c88e3421a70c7ca87cfea370
>>>>> Author:     Ryan Libby <rlibby@FreeBSD.org>
>>>>> AuthorDate: 2021-01-11 05:53:15 +0000
>>>>> Commit:     Ryan Libby <rlibby@FreeBSD.org>
>>>>> CommitDate: 2021-01-11 05:53:15 +0000
>>>>>
>>>>>    hid: quiet -Wswitch
>>>>>
>>>>>    Gcc builds complained that not all switch cases are handled.  Add
>>>>>    default cases to appease gcc.
>>>>>
>>>>>    Reviewed by:    hselasky (previous version), wulf
>>>>>    Sponsored by:   Dell EMC Isilon
>>>>>    Differential Revision:  https://reviews.freebsd.org/D28082
>>>>
>>>> If these cases are never reachable, then I think '__assert_unreachable()'
>>>> is preferred to a plain break.
>>>>
>>> These cases are reachable. They are NOP steps of state machine.
>>
>> How many states are there? It might be better to document that using an
>> explicit set of case labels that just immediately break (and then
>> -Wswitch will help you in future in case you ever forget to update one
>> of the switch statements). Where possible -Wswitch is best fixed, not
>> silenced with default, though there are times when the latter is
>> preferable.
>>
>> Jess
>>
> 
> There are currently two other enum values, and four total.
> 
> I agree with your and John's points in general.  In this case, this was
> specifically discussed in review and reviewers requested use of default.
> From my perspective, if that's what maintainers prefer after having
> considered the options, use of default is acceptable.
> 

Skipping of this 2 steps requires addition of extra flags just to avoid
calling of NOP routine once at device_probe() and once at
device_detach(). I see no reason to add such a micro-optimization at
cost of simplicity.



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?e42a1337-d268-31b0-7c68-a677435491b2>