Re: [XEN PATCH v2 2/5] xen: export get_free_port

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

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.

That said, I am still OK with making the suggested change if that's what
you prefer.

Another alternative is simply to rename "get_free_port" to
"evtchn_allocate" (or something else to distinguish it from
evtchn_allocate_port).