Re: [Libguestfs] [libnbd PATCH 1/2] generator: Refactor handling of closures in unlocked functions

We have a memory leak when a function with a closure exits early prior to registering that closure through some path that will guarantee cleanup. The easiest way to fix it is to guarantee that once a closure is passed into a public API, it will be cleaned regardless of whether that API succeeds or fails. But to avoid cleaning the closure more than once, we need to refactor our internal handling, in order to track when a closure has been handed off for later cleaning. The easiest way to do this is to pass closures by reference to all internal functions, so that helpers in the call stack can modify the incoming pointer rather than their own copy. This patch is just preparatory, with no semantic change. The public API still passes closures by copy, but the generator then operates on the address of that closure through all internal nbd_unlocked_* functions, rather than making further copies through each additional function call. --- generator/C.ml | 35 ++++++++++++----------- generator/C.mli | 1 + lib/debug.c | 6 ++-- lib/opt.c | 26 ++++++++--------- lib/rw.c | 76 ++++++++++++++++++++++++------------------------- 5 files changed, 73 insertions(+), 71 deletions(-) diff --git a/generator/C.ml b/generator/C.ml index deb77fa..280b319 100644 --- a/generator/C.ml +++ b/generator/C.ml @@ -99,16 +99,17 @@ let rec name_of_arg = function | UInt64 n -> [n] let rec print_arg_list ?(wrap = false) ?maxcol ?handle ?types ?(parens = true) + ?(closure_mark) args optargs =

+ if parens then pr "("; + if wrap then + pr_wrap ?maxcol ',' + (fun () -> print_arg_list' ?handle ?types ?closure_mark args optargs) + else + print_arg_list' ?handle ?types ?closure_mark args optargs; + if parens then pr ")" + +and print_arg_list' ?(handle = false) ?(types = true) ?(closure_mark = "")

args optargs = | Closure { cbname; cbargs } -> if types then pr "nbd_%s_callback " cbname; - pr "%s_callback" cbname + pr "%s%s_callback" closure_mark cbname

@@ -385,7 +386,7 @@ let generate_lib_unlocked_h () = pr "\n"; List.iter ( fun (name, { args; optargs; ret }) -> - print_extern ~wrap:true ("unlocked_" ^ name) args optargs ret + print_extern ~wrap:true ~closure_mark:"*" ("unlocked_" ^ name) args optargs ret