[Libguestfs] [PATCH libnbd v3 2/6] generator: Add attribute((nonnull)) annotations to non-NULL parameters

[Richard W.M. Jones]

On Thu, Sep 29, 2022 at 09:08:26AM -0500, Eric Blake wrote:
> On Thu, Sep 29, 2022 at 02:42:05PM +0100, Richard W.M. Jones wrote:
> > > > +  (* 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).
> > > 
> ...
> > > > +  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?
> 
> Using a shorter name for testing:
> 
> $ cat foo.c
> #define my(...) __attribute__((__nonnull__(__VA_ARGS__)))
> extern void foo (char *a) my (1);
> extern void bar (char *a, char *b) my (1, 2);
> $ gcc -E foo.c
> # 0 "foo.c"
> # 0 "<built-in>"
> # 0 "<command-line>"
> # 1 "/usr/include/stdc-predef.h" 1 3 4
> # 0 "<command-line>" 2
> # 1 "foo.c"
> 
> extern void foo (char *a) __attribute__((__nonnull__(1)));
> extern void bar (char *a, char *b) __attribute__((__nonnull__(1, 2)));
> $ gcc -o foo.o -c foo.c
> $ # compiled, so we satisfied gcc's attribute syntax
> 
> and similarly,
> #define LIBNBD_ATTRIBUTE_NONNULL(...) /* no-op */
> when disabling the macro.

Oh easier than I thought.  I'll push a fix for that, thanks.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
libguestfs lets you edit virtual machines.  Supports shell scripting,
bindings from many languages.  http://libguestfs.org