[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH] do_multicall and MISRA Rule 8.3
On Mon, 18 Mar 2024, Jan Beulich wrote: > The named reasons simply aren't convincing to me, at all. There's no > loss towards the "end goals" when "unsigned int" is used instead of > "uint32_t". Plus I recall Andrew putting under question use of > "unsigned int" for various hypercall parameter declarations, indicating > his belief that "unsigned long" ought to be used. That's, to me, quite > the opposite of using fixed-width types here, as there's no uniformly > correct fixed-width type to use in that case. > > So to me, besides there not having been technical arguments towards > the need to use fixed width types here, there's actually added > confusion about what's actually wanted. Imo if we started using fixed- > width types for hypercall handler parameter declarations, (almost?) all > non-handle ones would likely want to be converted. Consistency, after > all, is part of the "easy to maintain code" goal. Plus without > consistency how would one determine when to use what kind of types. [...] > The main use of fixed width types, to me, is in interface structure > definitions - between Xen and hardware / firmware, or in hypercall > structures. I'm afraid I have a hard time seeing good uses outside of > that. Even in inline assembly, types of variables used for inputs or > outputs don't strictly need to be fixed-width; I can somewhat accept > their use there for documentation purposes. Non-ABI interfaces are OK with native types. Our ABI interfaces are, for better or for worse, described using C header files. Looking at these header files, it should be clear the size and alignment of all integer parameters. To that end, I think we should use fixed-width types in all ABIs, including hypercall entry points. In my opinion, C hypercall entry points are part of the ABI and should match the integer types used in the public header files. I don't consider the little assembly code on hypercall entry as important. I think we should avoid having one description of the hypercall types in sched.h and different types used in do_sched_op. Sometimes, we might have parameters that vary in size depending on the architecture. For instance, a parameter could be 32-bit for 32-bit architectures and 64-bit for 64-bit architectures. For these cases, using "unsigned long", together with a document like the one I submitted recently to xen-devel, is a good way forward: it is semantically correct on all architectures, and it still comes with a precise size and alignment as described in the document. In this context, "unsigned long" means "register size" (on ARM we have register_t). This is the one case where it makes sense not to use a fixed-width type. Alternatively we would have to use #ifdefs.
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |