Re: [PATCH for-4.6 12/13] tools/ocaml: call libxl_dominfo_{init, dispose} in stub

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 23/07/15 10:55, Ian Campbell wrote:
> On Thu, 2015-07-23 at 09:32 +0100, Andrew Cooper wrote:
>> On 23/07/2015 08:59, Wei Liu wrote:
>>> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
>>> ---
>>> Cc: David Scott <dave.scott@eu.citrix.com>
>>>
>>> As far as I can tell, all Val_$foo function does deep-copy, so we 
>>> can
>>> safely call dispose in said function.
>> Sadly this is insufficient.  failwith_xl() longjump()s back into the
>> ocaml runtime, which ends up leaking any allocations made for 
>> dominfo.
>>
>> This is a systemic problem with the Ocaml bindings and I have a 
>> proposed
>> solution but it involves rewriting quite a lot of this code and is
>> definitely not 4.6 material.
> Is it not sufficient to treat failwith_xl as a longjump statement (or
> any sort of "return-y" thing), which would simply necessitate doing the
> cleanup before calling it?
>
> Perhaps Coverity could model it as such and would therefore warn about
> the dead code being added here?
>

Part of my Ocaml series is to properly mark failwith_xl() as a Noreturn
function.  Currently as far as the compiler and Coverity can tell,
failwith_xl() may return normally.

While it is possible to rearrange this code to avoid leaking in the ret
!= 0 case, it is not possible to rearrange it to avoid leaking if
Val_dominfo() uses failwith_xl()/caml_out_of_memory() itself.

The solution I have in mind is to wrap all libxl IDL objects in Ocaml
Custom blocks, which allows the Ocaml runtime to garbage collect them.

~Andrew