On Mon, 28 Aug 2023, Jan Beulich wrote: > On 26.08.2023 00:21, Stefano Stabellini wrote: > > Coming to unsigned int, it should be 32-bit on all supported arches, > > But this isn't a requirement, even if we're using "unsigned int" in the > C declarations / definitions: If "unsigned int" was wider, all we'd > demand is that hypercall entry code (likely written in assembly anyway) > zero-extend incoming values suitably to fit whatever "unsigned int" is. > Which is no different to Andrew (aiui) suggesting to use "unsigned > long" instead: That'll too require suitable zero-extension up front.
What you wrote assumes that "unsigned int" can only be 32-bit or wider, such as 64-bit. However, I think that by the C standard there is no guarantee. For instance, it could be smaller, e.g. 16-bit. There is also another assumption: if "unsigned int" was indeed 64-bit we could detect that a guest is 32-bit, assume that a 32-bit guest would interpret "unsigned int" as 32-bit and only pass 32-bit, and zero-extend the rest. I don't think it is a good idea to make unwritten assumption about the "unsigned int" size of the guest and as you know different OSes can assign different sizes to the same C type. This is why I think that in general if we wanted to use non-fixed-width as part of an ABI we would need to do a better job at writing down all of these assumptions. Without anything written down, it is easier to use fixed-width types. That said, in this case, we have been using "unsigned int" for years and it is fixed width for all the arches we support, so I think we should continue to use it for consistency, but ideally also help improve the documentation to write down all the unwritten assumptions we rely on. > > so > > it is down to consistency, which is a bit arbitrary. We have quite a > > good mix of unsigned int and uint32_t in hypercall-defs.c, specifically: > > 10 uint32_t > > 32 unsigned int > > > > By popular vote, I would go with unsigned int. So, I would keep the > > patch as is. > > Well, I wouldn't quite say "as is": It clearly wants splitting for the > two entirely unrelated changes. Then the uncontroversial part can go > in right away, while we settle on the controversial aspect. OK from me > As to "popular vote" - ./CODING_STYLE also matters here, and favors > non-fixed-width types over fixed-width ones. And as per above imo > there's no technical reason to use fixed-width types here. Yet I > understand Andrew takes a different perspective ... I think internal interfaces and public ABIs can have different policies and I understand the point that a public ABI should use only fixed-width types. That's not where we are today, but I would understand that argument.
