On Wed, Sep 28, 2022 at 12:47:24PM +0200, Laszlo Ersek wrote:
> 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?
No that's a mistake.
I was actually trying to think of a way where I don't need to list
these out in two places, but I couldn't think of how to do it except
to use polymorphic variants (`Variants).
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-top is 'top' for virtual machines. Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://people.redhat.com/~rjones/virt-top
_______________________________________________
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs
