On 02/12/2022 10:55, Edwin Török wrote:
> diff --git a/tools/ocaml/libs/xc/xenctrl_stubs.c
> b/tools/ocaml/libs/xc/xenctrl_stubs.c
> index d30585f21c..a492ea17fd 100644
> --- a/tools/ocaml/libs/xc/xenctrl_stubs.c
> +++ b/tools/ocaml/libs/xc/xenctrl_stubs.c
> @@ -641,6 +645,69 @@ CAMLprim value stub_xc_evtchn_reset(value xch, value
> domid)
> CAMLreturn(Val_unit);
> }
>
> +CAMLprim value stub_xc_evtchn_status(value xch, value domid, value port)
> +{
> + CAMLparam3(xch, domid, port);
> + CAMLlocal4(result, result_status, stat, interdomain);
> + xc_evtchn_status_t status = {
> + .dom = _D(domid),
> + .port = Int_val(port),
> + };
> + int rc;
> +
> + caml_enter_blocking_section();
> + rc = xc_evtchn_status(_H(xch), &status);
> + caml_leave_blocking_section();
> +
> + if ( rc < 0 )
> + failwith_xc(_H(xch));
> +
> + if ( status.status == EVTCHNSTAT_closed )
> + CAMLreturn(Val_none);
> +
> + switch ( status.status )
> + {
The EVTCHNSTAT_closed wants to be a case here, otherwise it's really
weird to read from a C point of view.
It would be fine to have a comment like this:
case EVTCHNSTAT_closed:
CAMLreturn(Val_none); /* Early exit, no allocations needed. */
to help identify more clearly that this a bit of a special case.
> + case EVTCHNSTAT_unbound:
> + stat = caml_alloc(1, 0); /* 1st non-constant constructor */
> + Store_field(stat, 0, Val_int(status.u.unbound.dom));
> + break;
> +
> + case EVTCHNSTAT_interdomain:
> + interdomain = caml_alloc_tuple(2);
> + Store_field(interdomain, 0, Val_int(status.u.interdomain.dom));
> + Store_field(interdomain, 1, Val_int(status.u.interdomain.port));
> + stat = caml_alloc(1, 1); /* 2nd non-constant constructor */
> + Store_field(stat, 0, interdomain);
> + break;
Newline here.
> + case EVTCHNSTAT_pirq:
> + stat = caml_alloc(1, 2); /* 3rd non-constant constructor */
> + Store_field(stat, 0, Val_int(status.u.pirq));
> + break;
> +
> + case EVTCHNSTAT_virq:
> + stat = caml_alloc(1, 3); /* 4th non-constant constructor */
> + Store_field(stat, 0, Val_int(status.u.virq));
> + break;
> +
> + case EVTCHNSTAT_ipi:
> + stat = Val_int(0); /* 1st constant constructor */
> + break;
> +
> + default:
> + caml_failwith("Unknown evtchn status");
> +
> + }
> + result_status = caml_alloc_tuple(2);
> + Store_field(result_status, 0, Val_int(status.vcpu));
> + Store_field(result_status, 1, stat);
> +
> + /* caml_alloc_some is missing in older versions of OCaml
> + */
I'd just drop this comment. It's going to be many many years before
Ocaml 4.12 drops off the bottom of the support list, so this observation
is unactionable.
All 3 of these are trivial to fix on commit, so Reviewed-by: Andrew
Cooper <[email protected]> otherwise.
~Andrew
