On 14.01.2022 02:20, Stefano Stabellini wrote: > On Thu, 13 Jan 2022, Jan Beulich wrote: >> On 13.01.2022 01:58, Stefano Stabellini wrote: >>> --- a/xen/common/event_channel.c >>> +++ b/xen/common/event_channel.c >>> @@ -232,7 +232,7 @@ int evtchn_allocate_port(struct domain *d, >>> evtchn_port_t port) >>> return 0; >>> } >>> >>> -static int get_free_port(struct domain *d) >>> +int get_free_port(struct domain *d) >> >> The name of the function isn't really suitable for being non-static. >> Can't we fold its functionality back into evtchn_allocate_port() (or >> the other way around, depending on the perspective you want to take) >> in case the caller passes in port 0? (Btw., it is imo wrong for the >> loop over ports to start at 0, when it is part of the ABI that port >> 0 is always invalid. evtchn_init() also better wouldn't depend on it >> being the only party to successfully invoke the function getting back >> port 0.) > > I agree that "get_free_port" is not a great name for a non-static > function. Also, it should be noted that for the sake of this patch > series I could just call evtchn_allocate_port(d, 1) given that it is the > first evtchn to be allocated. So maybe, that's the best way forward? > > > To address your specific suggestion, in my opinion it would be best to > have two different functions to allocate a new port: > - one with a specific evtchn_port_t port parameter > - one without it (meaning: "I don't care about the number") > > Folding the functionality of "give me any number" when 0 is passed to > evtchn_allocate_port() doesn't seem to be an improvement to the API to > me.
I view it the other way around - that way the function would actually start matching its name. So far it's more like marking a given port number as in use, rather than allocating. > That said, I am still OK with making the suggested change if that's what > you prefer. Given experience, hoping for others to voice an opinion isn't likely to become reality. Jan
