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

[Nicola Vetrini <nicola.vetrini@bugseng.com>]

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@bugseng.com>
>>> ---
>>> 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)