[Libguestfs] [PATCH libnbd 4/6] lib: Check Closure parameter is not NULL.

Eric Blake eblake at redhat.com
Tue Aug 13 11:26:31 UTC 2019


On 8/13/19 5:06 AM, Richard W.M. Jones wrote:
> This was not permitted by the API before, but would in some
> circumstances work.
> ---
>  generator/generator | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 

Requires nbd_pread_structured and nbd_block_status to pass in a callback
(good).  Requires nbd_set_debug_callback to provide a non-NULL pointer
(a bit rough, as now you can't pass NULL to deinstall your handler and
get back to the default behavior - but how often are applications likely
to deinstall a debug handler? If it becomes a problem, we can improve
the 'closure' type to take a bool argument stating whether the closure
accepts NULL, defaulting to false, but set to true for debug_closure -
or we could rewrite nbd_set_debug_callback to take an OClosure instead
of a Closure).  Requires completion_callback to be non-NULL, but we
relax that restriction by moving to OClosure.

ACK.

Yes, the first four can be applied no matter what else we discuss.

> diff --git a/generator/generator b/generator/generator
> index 7f97163..01da1c3 100755
> --- a/generator/generator
> +++ b/generator/generator
> @@ -3664,6 +3664,16 @@ let generate_lib_api_c () =
>      in
>      List.iter (
>        function
> +      | Closure { cbname } ->
> +         let value = match errcode with
> +           | Some value -> value
> +           | None -> assert false in

The generator has a check that:

    (* !may_set_error is incompatible with String parameters because
     * we have to do a NULL-check on those which may return an error.
     *)

Should this check be widened to cover Closure, so that we get a nicer
error than 'assert false' if someone goofs in adding a new signature
that uses Closure with the wrong return type?  Can be a separate patch,
since we missed doing it for Enum and Flags in earlier patches, which
also are instances where we used 'assert false' and require the ability
to return an error.  Doesn't affect runtime correctness, just
ease-of-use when re-compiling changes to the generator.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libguestfs/attachments/20190813/075ca141/attachment.sig>


More information about the Libguestfs mailing list