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

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
-- 
2.37.0.rc2

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