Re: [XEN RFC] x86/uaccess: remove __{put,get}_user_bad()

[Nicola Vetrini <nicola.vetrini@xxxxxxxxxxx>]

Hi Andrew,

On 2023-12-21 12:03, Andrew Cooper wrote:
> On 21/12/2023 10:58 am, Jan Beulich wrote:
> > On 21.12.2023 11:53, Federico Serafini wrote:
> > > Remove declarations of __put_user_bad() and __get_user_bad()
> > > since they have no definition.
> > > Replace their uses with a break statement to address violations of
> > > MISRA C:2012 Rule 16.3 ("An unconditional `break' statement shall
> > > terminate every switch-clause").
> > > No functional change.
> > >
> > > Signed-off-by: Federico Serafini <federico.serafini@xxxxxxxxxxx>
> > > ---
> > > Several violations of Rule 16.3 come from uses of macros
> > > get_unsafe_size() and put_unsafe_size().
> > > Looking at the macro definitions I found __get_user_bad() and 
> > > __put_user_bad().
> > > I was wondering if instead of just adding the break statement I can 
> > > also remove
> > > such functions which seem to not have a definition.
> > No, you can't. Try introducing a caller which "accidentally" uses the
> > wrong size. Without your change you'll observe the build failing (in
> > a somewhat obscure way, but still), while with your change bad code
> > will silently be generated.
> The construct here is deliberate.  It's a build time assertion that bad
> sizes aren't used.
>
> __bitop_bad_size() and __xsm_action_mismatch_detected() are the same
> pattern in other areas of code too, with the latter being more explicit
> because of how it's wrapped by LINKER_BUG_ON().
>
>
> It is slightly horrible, and not the most obvious construct for
> newcomers.  If there's an alternative way to get a build assertion, we
> could consider switching to a new pattern.
>
> ~Andrew
would you be in favour of a solution with a BUILD_BUG_ON in the default 
branch followed by a break?
default:
    BUILD_BUG_ON(!size || size >=8 || (size & (size - 1)));
    break;

--
Nicola Vetrini, BSc
Software Engineer, BUGSENG srl (https://bugseng.com)