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? Laszlo