Richard W.M. Jones
2022-Sep-29 13:42 UTC
[Libguestfs] [PATCH libnbd v3 2/6] generator: Add attribute((nonnull)) annotations to non-NULL parameters
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 <lersek at 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
Eric Blake
2022-Sep-29 14:08 UTC
[Libguestfs] [PATCH libnbd v3 2/6] generator: Add attribute((nonnull)) annotations to non-NULL parameters
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. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org