On 05.07.2022 17:22, Julien Grall wrote:
> Hi Jan,
>
> On 05/07/2022 16:11, Jan Beulich wrote:
>> On 22.06.2022 16:38, Rahul Singh wrote:
>>> @@ -387,8 +392,19 @@ int evtchn_bind_interdomain(evtchn_bind_interdomain_t
>>> *bind)
>>> spin_lock(&ld->event_lock);
>>> }
>>>
>>> - if ( (lport = get_free_port(ld)) < 0 )
>>> - ERROR_EXIT(lport);
>>> + if ( lport != 0 )
>>> + {
>>> + if ( (rc = evtchn_allocate_port(ld, lport)) != 0 )
>>> + ERROR_EXIT_DOM(rc, ld);
>>> + }
>>> + else
>>> + {
>>> + int alloc_port = get_free_port(ld);
>>> +
>>> + if ( alloc_port < 0 )
>>> + ERROR_EXIT_DOM(alloc_port, ld);
>>> + lport = alloc_port;
>>> + }
>>
>> This is then the 3rd instance of this pattern. I think the logic
>> wants moving into get_free_port() (perhaps with a name change).
>
> I think the code below would be suitable. I can send it or Rahul can
> re-use the commit [1]:
Ah yes; probably makes sense (right now) only in the context of his
series.
> --- a/xen/common/event_channel.c
> +++ b/xen/common/event_channel.c
> @@ -292,6 +292,18 @@ void evtchn_free(struct domain *d, struct evtchn *chn)
> xsm_evtchn_close_post(chn);
> }
>
> +static int evtchn_get_port(struct domain *d, uint32_t port)
Preferably evtchn_port_t.
> +{
> + int rc;
> +
> + if ( port != 0 )
> + rc = evtchn_allocate_port(d, port);
> + else
> + rc = get_free_port(d);
> +
> + return rc != 0 ? rc : port;
We commonly use "rc ?: port" in such cases. At which point I'd be
inclined to make it just
static int evtchn_get_port(struct domain *d, evtchn_port_t port)
{
return (port ? evtchn_allocate_port(d, port)
: get_free_port(d)) ?: port;
}
But I can see you or others having reservations against such ...
Jan
