Il giorno ven 7 lug 2023 alle ore 23:53 Stefano Stabellini < sstabell...@kernel.org> ha scritto:
> 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 <jbeul...@suse.com> > ha > > > scritto: > > > > > >> On 07.07.2023 08:50, Simone Ballarin wrote: > > >>> Il giorno gio 6 lug 2023 alle ore 18:22 Jan Beulich < > jbeul...@suse.com> > > >> ha > > >>> scritto: > > >>> > > >>>> On 06.07.2023 18:08, Simone Ballarin wrote: > > >>>>> Il giorno gio 6 lug 2023 alle ore 10:26 Jan Beulich < > jbeul...@suse.com > > >>> > > >>>> 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. > I agree. -- Simone Ballarin, M.Sc. Field Application Engineer, BUGSENG (https://bugseng.com <http://bugseng.com>)
