On Thu, Sep 29, 2022 at 08:25:29AM -0500, Eric Blake wrote:
> On Wed, Sep 28, 2022 at 06:25:35PM +0100, Richard W.M. Jones wrote:
> > For API parameters that are pointers and must not be NULL, add the
> > appropriate GCC annotations.
> >
> > Reviewed-by: Laszlo Ersek <ler...@redhat.com>
> > ---
> > generator/C.ml | 59 +++++++++++++++++++++++++++++++++++--
> > tests/errors-connect-null.c | 4 +++
> > 2 files changed, 61 insertions(+), 2 deletions(-)
>
> > @@ -216,7 +238,21 @@ let
> > let print_fndecl ?wrap ?closure_style name args optargs ret =
> > pr "extern ";
> > print_call ?wrap ?closure_style name args optargs ret;
> > - pr ";\n"
> > +
> > + (* Output __attribute__((nonnull)) for the function parameters:
> > + * eg. struct nbd_handle *, int, char *
> > + * => [ true, false, true ]
> > + * => LIBNBD_ATTRIBUTE_NONNULL((1,3))
> > + * => __attribute__((nonnull,(1,3)))
>
> Style question. Do we want to REQUIRE the clients of this macro to
> pass in (), or would it be better to have a varargs format?
>
> > + *)
> > + let nns : bool list =
> > + [ true ] (* struct nbd_handle * *)
> > + @ List.flatten (List.map arg_attr_nonnull args)
> > + @ List.flatten (List.map optarg_attr_nonnull optargs) in
> > + let nns = List.mapi (fun i b -> (i+1, b)) nns in
> > + let nns = filter_map (fun (i, b) -> if b then Some i else None) nns in
> > + let nns : string list = List.map string_of_int nns in
> > + pr "\n LIBNBD_ATTRIBUTE_NONNULL((%s));\n" (String.concat "," nns)
>
> For generated code, it is just as easy to cope with either style (we
> can strip a layer of () if we want a varargs format).
>
> >
> > let rec print_cbarg_list ?(wrap = false) ?maxcol ?types ?(parens = true)
> > cbargs =
> > @@ -349,6 +385,19 @@ let
> > pr "extern \"C\" {\n";
> > pr "#endif\n";
> > pr "\n";
> > + pr "#if defined(__GNUC__)\n";
> > + pr "#define LIBNBD_GCC_VERSION \\\n";
> > + pr " (__GNUC__ * 10000 + __GNUC_MINOR__ * 100 +
> > __GNUC_PATCHLEVEL__)\n";
> > + pr "#endif\n";
> > + pr "\n";
> > + pr "#ifndef LIBNBD_ATTRIBUTE_NONNULL\n";
> > + pr "#if defined(__GNUC__) && LIBNBD_GCC_VERSION >= 120000 /* gcc >= 12.0
> > */\n";
> > + pr "#define LIBNBD_ATTRIBUTE_NONNULL(s) __attribute__((__nonnull__
> > s))\n";
>
> This definition is what requires us to pass in our own (). That is,
> our end result is going to be one of:
>
> __attribute__((__nonnull__(1) ))
> __attribute__((__nonnull__(1, 2) ))
>
> but the difference is whether we must pass exactly one macro argument,
> and where that argument must include () even when there is only one
> parameter to be marked (what you coded):
>
> LIBNBD_ATTRIBUTE_NONNULL((1))
> LIBNBD_ATTRIBUTE_NONNULL((1, 3))
>
> vs. ease-of-use in supplying the () as part of the macro definition
> itself by using #define MACRO(...) and __VA_ARGS__:
>
> LIBNBD_ATTRIBUTE_NONNULL(1)
> LIBNBD_ATTRIBUTE_NONNULL(1, 3)
I'm not sure I understand - what does the second definition look like?
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
nbdkit - Flexible, fast NBD server with plugins
https://gitlab.com/nbdkit/nbdkit
_______________________________________________
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs
