Re: [Libguestfs] [PATCH libnbd v2 2/4] generator: Check that more parameters are not NULL

On 09/28/22 11:30, Richard W.M. Jones wrote: > We previously checked only that String parameters are not NULL, > returning an error + EFAULT if so. > > However we did not check Bytes*, SockAddrAndLen, Path or StringList > parameters, also never NULL. Be consistent about checks. > > Thanks: Eric Blake for help and an earlier version of the patch > --- > generator/API.ml | 7 +++++-- > generator/C.ml | 7 ++++++- > 2 files changed, 11 insertions(+), 3 deletions(-) > > diff --git a/generator/API.ml b/generator/API.ml > index 6e3213ad26..0975a407c1 100644 > --- a/generator/API.ml > +++ b/generator/API.ml > @@ -3851,13 +3851,16 @@ let () = > > (* !may_set_error is incompatible with certain parameters because > * we have to do a NULL-check on those which may return an error. > + * Refer to generator/C.ml:generator_lib_api_c. > *) > | name, { args; may_set_error = false } > when List.exists > (function > - | Closure _ | Enum _ | Flags _ | String _ -> true > + | Closure _ | Enum _ | Flags _ > + | BytesIn _ | BytesOut _ | BytesPersistIn _ | BytesPersistOut _ > + | SockAddrAndLen _ | Path _ | String _ -> true > | _ -> false) args -> > - failwithf "%s: if args contains Closure/Enum/Flags/String parameter, may_set_error must be false" name > + failwithf "%s: if args contains any non-null pointer parameter, may_set_error must be false" name > > (* !may_set_error is incompatible with certain optargs too. > *) > diff --git a/generator/C.ml b/generator/C.ml > index cafc5590e2..bfc216609e 100644 > --- a/generator/C.ml > +++ b/generator/C.ml > @@ -614,7 +614,12 @@ let > need_out_label := true > | Flags (n, flags) -> > print_flags_check n flags None > - | String n -> > + | BytesIn (n, _) | BytesOut (n, _) > + | BytesPersistIn (n, _) | BytesPersistOut (n, _) > + | SockAddrAndLen (n, _) > + | Path n > + | String n > + | StringList n -> > let value = match errcode with > | Some value -> value > | None -> assert false in > The first patch marks (some parts of the C-language projection of) parameter types BytesIn, BytesOut, BytesPersistIn, BytesPersistOut, SockAddrAndLen, Path, String, and StringList, as non-null. The second hunk in this patch covers all of those, but the first hunk does not cover StringList. Is that intentional?