From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from lists.xenproject.org (lists.xenproject.org [192.237.175.120]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 49C07EB64D9 for ; Fri, 7 Jul 2023 21:54:18 +0000 (UTC) Received: from list by lists.xenproject.org with outflank-mailman.560624.876684 (Exim 4.92) (envelope-from ) id 1qHtOm-0000bu-BA; Fri, 07 Jul 2023 21:53:56 +0000 X-Outflank-Mailman: Message body and most headers restored to incoming version Received: by outflank-mailman (output) from mailman id 560624.876684; Fri, 07 Jul 2023 21:53:56 +0000 Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1qHtOm-0000bn-7u; Fri, 07 Jul 2023 21:53:56 +0000 Received: by outflank-mailman (input) for mailman id 560624; Fri, 07 Jul 2023 21:53:54 +0000 Received: from se1-gles-flk1-in.inumbo.com ([94.247.172.50] helo=se1-gles-flk1.inumbo.com) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1qHtOk-0000bg-Bh for xen-devel@lists.xenproject.org; Fri, 07 Jul 2023 21:53:54 +0000 Received: from dfw.source.kernel.org (dfw.source.kernel.org [2604:1380:4641:c500::1]) by se1-gles-flk1.inumbo.com (Halon) with ESMTPS id c12b0ab5-1d10-11ee-8611-37d641c3527e; Fri, 07 Jul 2023 23:53:50 +0200 (CEST) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id D963861A8A; Fri, 7 Jul 2023 21:53:48 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 8F3BEC433C7; Fri, 7 Jul 2023 21:53:46 +0000 (UTC) X-BeenThere: xen-devel@lists.xenproject.org List-Id: Xen developer discussion List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: xen-devel-bounces@lists.xenproject.org Precedence: list Sender: "Xen-devel" X-Inumbo-ID: c12b0ab5-1d10-11ee-8611-37d641c3527e DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1688766828; bh=k/+e0BWi3gX9F1S1IBS6aojJc9k/vUACTYADyTfduqo=; h=Date:From:To:cc:Subject:In-Reply-To:References:From; b=s4Iy39WozA7wUOM/DuLT0e3iwDaTq10X1yw8ocBvgDZwrUu84RvVZ80TAHe8tSPF/ uW1KkLkupBGFNtU8lnsrMG6ZdEFAxhFo1XYJR9+WIaRxKuOtAfDNjDb9etYfrk/2RR 2Du5gyQyb0F8ALTqmAKqgiG2vQFu+lBlVNRdPY8Ftj5yoORyMc5JZhjAK0mQDQf2SQ WDlZuzZHpKaaOMvqkTIfezXMhmM8rJ8BRq/AHfquAu+huBZ71/7WwpHARjN3Et3Ali DtmQuWgrilnycxpK8cIRk0DF/JqRei0sw4NZS2WV354JntB9XetCWXLHiBeiVWV6T5 xFFfVIxYlqxbg== Date: Fri, 7 Jul 2023 14:53:45 -0700 (PDT) From: Stefano Stabellini X-X-Sender: sstabellini@ubuntu-linux-20-04-desktop To: Jan Beulich cc: Simone Ballarin , consulting@bugseng.com, Gianluca Luparini , Andrew Cooper , =?UTF-8?Q?Roger_Pau_Monn=C3=A9?= , Wei Liu , Paul Durrant , Stefano Stabellini , Michal Orzel , Xenia Ragiadakou , Ayan Kumar Halder , xen-devel@lists.xenproject.org Subject: Re: [XEN PATCH v2 12/13] xen/x86: fix violations of MISRA C:2012 Rule 7.2 In-Reply-To: Message-ID: References: User-Agent: Alpine 2.22 (DEB 394 2020-01-19) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII On Fri, 7 Jul 2023, Jan Beulich wrote: > On 07.07.2023 10:04, Simone Ballarin wrote: > > Il giorno ven 7 lug 2023 alle ore 09:04 Jan Beulich ha > > scritto: > > > >> On 07.07.2023 08:50, Simone Ballarin wrote: > >>> Il giorno gio 6 lug 2023 alle ore 18:22 Jan Beulich > >> ha > >>> scritto: > >>> > >>>> On 06.07.2023 18:08, Simone Ballarin wrote: > >>>>> Il giorno gio 6 lug 2023 alle ore 10:26 Jan Beulich >>> > >>>> ha > >>>>> scritto: > >>>>> > >>>>>> On 05.07.2023 17:26, Simone Ballarin wrote: > >>>>>>> --- a/xen/arch/x86/apic.c > >>>>>>> +++ b/xen/arch/x86/apic.c > >>>>>>> @@ -1211,7 +1211,7 @@ static void __init calibrate_APIC_clock(void) > >>>>>>> * Setup the APIC counter to maximum. There is no way the lapic > >>>>>>> * can underflow in the 100ms detection time frame. > >>>>>>> */ > >>>>>>> - __setup_APIC_LVTT(0xffffffff); > >>>>>>> + __setup_APIC_LVTT(0xffffffffU); > >>>>>> > >>>>>> While making the change less mechanical, we want to consider to switch > >>>>>> to ~0 in this and similar cases. > >>>>>> > >>>>> > >>>>> Changing ~0U is more than not mechanical: it is possibly dangerous. > >>>>> The resulting value could be different depending on the architecture, > >>>>> I prefer to not make such kind of changes in a MISRA-related patch. > >>>> > >>>> What do you mean by "depending on the architecture", when this is > >>>> x86-only code _and_ you can check what type parameter the called > >>>> function has? > >>> > >>> Ok, I will change these literals in ~0U in the next submission. > >> > >> Except that I specifically meant ~0, not ~0U. We mean "maximum value" > >> here, and at the call site it doesn't matter how wide the function > >> parameter's type is. If it was 64-bit, ~0U would not do what is wanted. > > > > ~0 is not a MISRA-compliant solution since bitwise operations on signed > > integers have implementation-defined behavior. This solution definitively > > violates Rule 10.1. > > So if we adopted that rule (we didn't so far), we'd have to e.g. change > all literal number shift counts to have U suffixes, no matter that > without the suffix it is still entirely obvious that the numbers are > unsigned? I'm afraid that'll face my opposition ... Indeed we have not adopted Rule 10.1. However, may I suggest that we don't make things potentially worse, just in case we end up deciding in favor of 10.1? We might not adopt 10.1 at all, but still... The code is already 0xffffffff, let's make things easier for all of us and just do 0xffffffffU ? Let's put Rule 10.1 and the whole of MISRA C aside for a second. Jan, let's say that you prefer ~0 or a different function parameter name or something else on any of these patches. You do realize that you don't need Simone or Federico or anyone else to make that change for you? You can make the change, submit a patch, and in your case anyone can ack it. Roger, Andrew, me, Bertrand, Julien, and almost anyone else could ack it and it would go in. As I wrote yesterday, feel free to CC me and I'll help you get in all the changes that you want. If you submitted that patch to switch to ~0 it might already be committed by now. I am trying to highlight that suggesting changes on these mechanical patches end up with more work for both the contributor and also the maintainer compared to do the change yourself. I think we should try to accept these patches as mechanical-changes-only. This is the only way to scale up this effort. If you spot something that you'd rather be done differently, do one of the following: a) Accept the patch as-is and submit a patch afterwards. Yes the line gets changed twice but it is the easiest solution. b) Ask the contribitor to drop the single change you would rather do differently, or even better drop it yourself on commit. Then submit a patch with the change that you prefer. c) For trivial things, like code style changes, do the change directly on commit. I know emails encourage English replies, but to make this work we need to do more code changes together and less English.