[Qemu-devel] [PATCH v2] add macro file for coccinelle

[Qemu-devel] [PATCH v2] add macro file for coccinelle

[Paolo Bonzini]

Coccinelle chokes on some idioms from compiler.h and queue.h.
Extract those in a macro file, to be used with "--macro-file
scripts/cocci-macro-file.h".

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 scripts/cocci-macro-file.h | 119 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 119 insertions(+)
 create mode 100644 scripts/cocci-macro-file.h

diff --git a/scripts/cocci-macro-file.h b/scripts/cocci-macro-file.h
new file mode 100644
index 0000000..eceb4be
--- /dev/null
+++ b/scripts/cocci-macro-file.h
@@ -0,0 +1,119 @@
+/* Macro file for Coccinelle
+ *
+ * Copyright (C) 2015 Red Hat, Inc.
+ *
+ * Authors:
+ *  Paolo Bonzini <pbonzini@redhat.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or, at your
+ * option, any later version.  See the COPYING file in the top-level directory.
+ */
+
+/* Coccinelle only does limited parsing of headers, and chokes on some idioms
+ * defined in compiler.h and queue.h.  Macros that Coccinelle must know about
+ * in order to parse .c files must be in a separate macro file---which is
+ * exactly what you're staring at now.
+ *
+ * To use this file, add the "--macro-file scripts/cocci-macro-file.h" to the
+ * Coccinelle command line.
+ */
+
+/* From qemu/compiler.h */
+#define QEMU_GNUC_PREREQ(maj, min) 1
+#define QEMU_NORETURN __attribute__ ((__noreturn__))
+#define QEMU_WARN_UNUSED_RESULT __attribute__((warn_unused_result))
+#define QEMU_SENTINEL __attribute__((sentinel))
+#define QEMU_ARTIFICIAL __attribute__((always_inline, artificial))
+#define QEMU_PACKED __attribute__((gcc_struct, packed))
+
+#define cat(x,y) x ## y
+#define cat2(x,y) cat(x,y)
+#define QEMU_BUILD_BUG_ON(x) \
+    typedef char cat2(qemu_build_bug_on__,__LINE__)[(x)?-1:1] __attribute__((unused));
+
+#define GCC_FMT_ATTR(n, m) __attribute__((format(gnu_printf, n, m)))
+
+#define xglue(x, y) x ## y
+#define glue(x, y) xglue(x, y)
+#define stringify(s)	tostring(s)
+#define tostring(s)	#s
+
+#define typeof_field(type, field) typeof(((type *)0)->field)
+#define type_check(t1,t2) ((t1*)0 - (t2*)0)
+
+/* From qemu/queue.h */
+
+#define QLIST_HEAD(name, type)                                          \
+struct name {                                                           \
+        struct type *lh_first;  /* first element */                     \
+}
+
+#define QLIST_HEAD_INITIALIZER(head)                                    \
+        { NULL }
+
+#define QLIST_ENTRY(type)                                               \
+struct {                                                                \
+        struct type *le_next;   /* next element */                      \
+        struct type **le_prev;  /* address of previous next element */  \
+}
+
+/*
+ * Singly-linked List definitions.
+ */
+#define QSLIST_HEAD(name, type)                                          \
+struct name {                                                           \
+        struct type *slh_first; /* first element */                     \
+}
+
+#define QSLIST_HEAD_INITIALIZER(head)                                    \
+        { NULL }
+
+#define QSLIST_ENTRY(type)                                               \
+struct {                                                                \
+        struct type *sle_next;  /* next element */                      \
+}
+
+/*
+ * Simple queue definitions.
+ */
+#define QSIMPLEQ_HEAD(name, type)                                       \
+struct name {                                                           \
+    struct type *sqh_first;    /* first element */                      \
+    struct type **sqh_last;    /* addr of last next element */          \
+}
+
+#define QSIMPLEQ_HEAD_INITIALIZER(head)                                 \
+    { NULL, &(head).sqh_first }
+
+#define QSIMPLEQ_ENTRY(type)                                            \
+struct {                                                                \
+    struct type *sqe_next;    /* next element */                        \
+}
+
+/*
+ * Tail queue definitions.
+ */
+#define Q_TAILQ_HEAD(name, type, qual)                                  \
+struct name {                                                           \
+        qual type *tqh_first;           /* first element */             \
+        qual type *qual *tqh_last;      /* addr of last next element */ \
+}
+#define QTAILQ_HEAD(name, type)                                         \
+struct name {                                                           \
+        type *tqh_first;      /* first element */                       \
+        type **tqh_last;      /* addr of last next element */           \
+}
+
+#define QTAILQ_HEAD_INITIALIZER(head)                                   \
+        { NULL, &(head).tqh_first }
+
+#define Q_TAILQ_ENTRY(type, qual)                                       \
+struct {                                                                \
+        qual type *tqe_next;            /* next element */              \
+        qual type *qual *tqe_prev;      /* address of previous next element */\
+}
+#define QTAILQ_ENTRY(type)                                              \
+struct {                                                                \
+        type *tqe_next;       /* next element */                        \
+        type **tqe_prev;      /* address of previous next element */    \
+}
-- 
2.4.3

Re: [Qemu-devel] [PATCH v2] add macro file for coccinelle

[Markus Armbruster]

Paolo Bonzini <pbonzini@redhat.com> writes:

> Coccinelle chokes on some idioms from compiler.h and queue.h.
> Extract those in a macro file, to be used with "--macro-file
> scripts/cocci-macro-file.h".
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

I tested this as follows.  Coccinelle can report on its parsing
difficulties:

$ spatch --parse-c `git-ls-files | grep '\.c$'

Requires a generous ulimit -s, and produces tons of output.  The
interesting part here is the summary at the end:

    NB total files = 1923; perfect = 1377; pbs = 545; timeout = 0; =========> 71%
    nb good = 964472,  nb passed = 14824 =========> 1.48% passed
    nb good = 964472,  nb bad = 23759 =========> 97.63% good or passed


Repeat with --macro-file scripts/cocci-macro-file.h.

    NB total files = 1923; perfect = 1463; pbs = 459; timeout = 0; =========> 76%
    nb good = 910559,  nb passed = 15352 =========> 1.53% passed
    nb good = 910559,  nb bad = 77610 =========> 92.27% good or passed

The first line shows clear improvement: 86 more files are now deemed
"perfect".  I'm not sure how to read the remaining two lines.

Also interesting are the "maybe 10 most problematic tokens".  Before:

    -----------------------------------------------------------------------
    maybe 10 most problematic tokens
    -----------------------------------------------------------------------
    glue: present in 620 parsing errors
    example: 

           #define OP_32_64(x)                             \
                   glue(glue(case INDEX_op_, x), _i32):    \
                   glue(glue(case INDEX_op_, x), _i64)
    NULL: present in 507 parsing errors
    example: 
               vs->dev = (QVirtioDevice *)dev;
               g_assert(dev != NULL);
               g_assert_cmphex(vs->dev->device_type, ==, QVIRTIO_SCSI_DEVICE_ID);

    VMStateField: present in 502 parsing errors
    example: 
               .version_id = 1,
               .minimum_version_id = 1,
               .fields = (VMStateField[]) {
                   VMSTATE_VBUFFER_UINT32(data, Fifo8, 1, NULL, 0, capacity),
    fields: present in 502 parsing errors
    example: 
               .version_id = 1,
               .minimum_version_id = 1,
               .fields = (VMStateField[]) {
                   VMSTATE_VBUFFER_UINT32(data, Fifo8, 1, NULL, 0, capacity),
    minimum_version_id: present in 433 parsing errors
    example: 
               .version_id = 1,
               .minimum_version_id = 1,
               .fields = (VMStateField[]) {
                   VMSTATE_VBUFFER_UINT32(data, Fifo8, 1, NULL, 0, capacity),
    uint32_t: present in 414 parsing errors
    example: 
                                       uint32_t *bg, uint32_t *fg,                 \
                                       VncPalette **palette) {                     \
                   uint##bpp##_t *data;                                            \
                   uint##bpp##_t c0, c1, ci;                                       \
    env: present in 400 parsing errors
    example: 
               int v = float32_compare_quiet(a, b, &env->fp_status);
               set_br(env, v != float_relation_greater, br);
           }
    version_id: present in 308 parsing errors
    example: 
               .version_id = 1,
               .minimum_version_id = 1,
               .fields = (VMStateField[]) {
                   VMSTATE_VBUFFER_UINT32(data, Fifo8, 1, NULL, 0, capacity),
    x: present in 221 parsing errors
    example: 
               Int128 a = expand(x);
               Int128 r = int128_rshift(a, n);
               g_assert_cmpuint(r.lo, ==, l);
               g_assert_cmpuint(r.hi, ==, h);
    a: present in 197 parsing errors
    example: 
               vmstate_load_state(loading, &vmstate_skipping, &obj, 2);
               g_assert(!qemu_file_get_error(loading));
               g_assert_cmpint(obj.a, ==, 10);
               g_assert_cmpint(obj.b, ==, 20);

After:

    -----------------------------------------------------------------------
    maybe 10 most problematic tokens
    -----------------------------------------------------------------------
    NULL: present in 506 parsing errors
    example: 
               vs->dev = (QVirtioDevice *)dev;
               g_assert(dev != NULL);
               g_assert_cmphex(vs->dev->device_type, ==, QVIRTIO_SCSI_DEVICE_ID);

    VMStateField: present in 502 parsing errors
    example: 
               .version_id = 1,
               .minimum_version_id = 1,
               .fields = (VMStateField[]) {
                   VMSTATE_VBUFFER_UINT32(data, Fifo8, 1, NULL, 0, capacity),
    fields: present in 502 parsing errors
    example: 
               .version_id = 1,
               .minimum_version_id = 1,
               .fields = (VMStateField[]) {
                   VMSTATE_VBUFFER_UINT32(data, Fifo8, 1, NULL, 0, capacity),
    minimum_version_id: present in 433 parsing errors
    example: 
               .version_id = 1,
               .minimum_version_id = 1,
               .fields = (VMStateField[]) {
                   VMSTATE_VBUFFER_UINT32(data, Fifo8, 1, NULL, 0, capacity),
    env: present in 376 parsing errors
    example: 
               int v = float32_compare_quiet(a, b, &env->fp_status);
               set_br(env, v != float_relation_greater, br);
           }
    version_id: present in 308 parsing errors
    example: 
               .version_id = 1,
               .minimum_version_id = 1,
               .fields = (VMStateField[]) {
                   VMSTATE_VBUFFER_UINT32(data, Fifo8, 1, NULL, 0, capacity),
    x: present in 221 parsing errors
    example: 
               Int128 a = expand(x);
               Int128 r = int128_rshift(a, n);
               g_assert_cmpuint(r.lo, ==, l);
               g_assert_cmpuint(r.hi, ==, h);
    a: present in 197 parsing errors
    example: 
               vmstate_load_state(loading, &vmstate_skipping, &obj, 2);
               g_assert(!qemu_file_get_error(loading));
               g_assert_cmpint(obj.a, ==, 10);
               g_assert_cmpint(obj.b, ==, 20);
    b: present in 164 parsing errors
    example: 
                   uint64_t rl, rh;
                   muls64(&rl, &rh, test_s_data[i].a, test_s_data[i].b);
                   g_assert_cmpuint(rl, ==, test_s_data[i].rl);
                   g_assert_cmpint(rh, ==, test_s_data[i].rh);
    name: present in 150 parsing errors
    example: 
               g_assert(list != NULL);
               g_assert(QTAILQ_EMPTY(&list->head));
               g_assert_cmpstr(list->name, ==, "opts_list_01");

    -----------------------------------------------------------------------

Clearly visible is the effect of your cocci-macro-file.h's glue() .

> ---
>  scripts/cocci-macro-file.h | 119 +++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 119 insertions(+)
>  create mode 100644 scripts/cocci-macro-file.h
>
> diff --git a/scripts/cocci-macro-file.h b/scripts/cocci-macro-file.h
> new file mode 100644
> index 0000000..eceb4be
> --- /dev/null
> +++ b/scripts/cocci-macro-file.h
> @@ -0,0 +1,119 @@
> +/* Macro file for Coccinelle
> + *
> + * Copyright (C) 2015 Red Hat, Inc.
> + *
> + * Authors:
> + *  Paolo Bonzini <pbonzini@redhat.com>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or, at your
> + * option, any later version.  See the COPYING file in the top-level directory.
> + */
> +
> +/* Coccinelle only does limited parsing of headers, and chokes on some idioms
> + * defined in compiler.h and queue.h.  Macros that Coccinelle must know about
> + * in order to parse .c files must be in a separate macro file---which is
> + * exactly what you're staring at now.
> + *
> + * To use this file, add the "--macro-file scripts/cocci-macro-file.h" to the
> + * Coccinelle command line.
> + */
> +
> +/* From qemu/compiler.h */
> +#define QEMU_GNUC_PREREQ(maj, min) 1
> +#define QEMU_NORETURN __attribute__ ((__noreturn__))
> +#define QEMU_WARN_UNUSED_RESULT __attribute__((warn_unused_result))
> +#define QEMU_SENTINEL __attribute__((sentinel))
> +#define QEMU_ARTIFICIAL __attribute__((always_inline, artificial))
> +#define QEMU_PACKED __attribute__((gcc_struct, packed))
> +
> +#define cat(x,y) x ## y
> +#define cat2(x,y) cat(x,y)

Why not reuse glue()?

> +#define QEMU_BUILD_BUG_ON(x) \
> + typedef char cat2(qemu_build_bug_on__,__LINE__)[(x)?-1:1]
> __attribute__((unused));
> +
> +#define GCC_FMT_ATTR(n, m) __attribute__((format(gnu_printf, n, m)))
> +
> +#define xglue(x, y) x ## y
> +#define glue(x, y) xglue(x, y)
> +#define stringify(s)	tostring(s)
> +#define tostring(s)	#s
> +
> +#define typeof_field(type, field) typeof(((type *)0)->field)
> +#define type_check(t1,t2) ((t1*)0 - (t2*)0)
> +
[...]

Preferably with cat2() replaced by glue():
Reviewed-by: Markus Armbruster <armbru@redhat.com>

Re: [Qemu-devel] [PATCH v2] add macro file for coccinelle

[Paolo Bonzini]

> > +/* From qemu/compiler.h */
> > +#define QEMU_GNUC_PREREQ(maj, min) 1
> > +#define QEMU_NORETURN __attribute__ ((__noreturn__))
> > +#define QEMU_WARN_UNUSED_RESULT __attribute__((warn_unused_result))
> > +#define QEMU_SENTINEL __attribute__((sentinel))
> > +#define QEMU_ARTIFICIAL __attribute__((always_inline, artificial))
> > +#define QEMU_PACKED __attribute__((gcc_struct, packed))
> > +
> > +#define cat(x,y) x ## y
> > +#define cat2(x,y) cat(x,y)
> 
> Why not reuse glue()?

Because the duplication is already in compiler.h, which should
use glue() instead of cat2().  Separate patch IMHO, but thanks
for noticing. :-)

Paolo

> > +#define QEMU_BUILD_BUG_ON(x) \
> > + typedef char cat2(qemu_build_bug_on__,__LINE__)[(x)?-1:1]
> > __attribute__((unused));
> > +
> > +#define GCC_FMT_ATTR(n, m) __attribute__((format(gnu_printf, n, m)))
> > +
> > +#define xglue(x, y) x ## y
> > +#define glue(x, y) xglue(x, y)
> > +#define stringify(s)	tostring(s)
> > +#define tostring(s)	#s
> > +
> > +#define typeof_field(type, field) typeof(((type *)0)->field)
> > +#define type_check(t1,t2) ((t1*)0 - (t2*)0)
> > +
> [...]
> 
> Preferably with cat2() replaced by glue():
> Reviewed-by: Markus Armbruster <armbru@redhat.com>

Re: [Qemu-devel] [PATCH v2] add macro file for coccinelle

[Markus Armbruster]

Paolo Bonzini <pbonzini@redhat.com> writes:

>> > +/* From qemu/compiler.h */
>> > +#define QEMU_GNUC_PREREQ(maj, min) 1
>> > +#define QEMU_NORETURN __attribute__ ((__noreturn__))
>> > +#define QEMU_WARN_UNUSED_RESULT __attribute__((warn_unused_result))
>> > +#define QEMU_SENTINEL __attribute__((sentinel))
>> > +#define QEMU_ARTIFICIAL __attribute__((always_inline, artificial))
>> > +#define QEMU_PACKED __attribute__((gcc_struct, packed))
>> > +
>> > +#define cat(x,y) x ## y
>> > +#define cat2(x,y) cat(x,y)
>> 
>> Why not reuse glue()?
>
> Because the duplication is already in compiler.h, which should
> use glue() instead of cat2().  Separate patch IMHO, but thanks
> for noticing. :-)

The separate patch is commit 24134c4 :)

Re: [Qemu-devel] [Qemu-trivial] [PATCH v2] add macro file for coccinelle

[Marcel Apfelbaum]

On 09/08/2015 02:10 PM, Markus Armbruster wrote:
> Paolo Bonzini <pbonzini@redhat.com> writes:
>
>> Coccinelle chokes on some idioms from compiler.h and queue.h.
>> Extract those in a macro file, to be used with "--macro-file
>> scripts/cocci-macro-file.h".
>>
>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>
> I tested this as follows.  Coccinelle can report on its parsing
> difficulties:
>
> $ spatch --parse-c `git-ls-files | grep '\.c$'
>
> Requires a generous ulimit -s, and produces tons of output.  The
> interesting part here is the summary at the end:
>
>      NB total files = 1923; perfect = 1377; pbs = 545; timeout = 0; =========> 71%
>      nb good = 964472,  nb passed = 14824 =========> 1.48% passed
>      nb good = 964472,  nb bad = 23759 =========> 97.63% good or passed
>
>
> Repeat with --macro-file scripts/cocci-macro-file.h.
>
>      NB total files = 1923; perfect = 1463; pbs = 459; timeout = 0; =========> 76%
>      nb good = 910559,  nb passed = 15352 =========> 1.53% passed
>      nb good = 910559,  nb bad = 77610 =========> 92.27% good or passed
>
> The first line shows clear improvement: 86 more files are now deemed
> "perfect".  I'm not sure how to read the remaining two lines.
>
> Also interesting are the "maybe 10 most problematic tokens".  Before:
>
>      -----------------------------------------------------------------------
>      maybe 10 most problematic tokens
>      -----------------------------------------------------------------------
>      glue: present in 620 parsing errors
>      example:
>
>             #define OP_32_64(x)                             \
>                     glue(glue(case INDEX_op_, x), _i32):    \
>                     glue(glue(case INDEX_op_, x), _i64)
>      NULL: present in 507 parsing errors
>      example:
>                 vs->dev = (QVirtioDevice *)dev;
>                 g_assert(dev != NULL);
>                 g_assert_cmphex(vs->dev->device_type, ==, QVIRTIO_SCSI_DEVICE_ID);
>
>      VMStateField: present in 502 parsing errors
>      example:
>                 .version_id = 1,
>                 .minimum_version_id = 1,
>                 .fields = (VMStateField[]) {
>                     VMSTATE_VBUFFER_UINT32(data, Fifo8, 1, NULL, 0, capacity),

Hi Markus,

I talked to coccinelle maintainer some time ago about
the above C construct (fields = (VMStateField[]) {)
and sadly is not yet supported by the C parser.

If we want this we need to do it by ourselves :(

Thanks,
Marcel


>      fields: present in 502 parsing errors
>      example:
>                 .version_id = 1,
>                 .minimum_version_id = 1,
>                 .fields = (VMStateField[]) {
>                     VMSTATE_VBUFFER_UINT32(data, Fifo8, 1, NULL, 0, capacity),
>      minimum_version_id: present in 433 parsing errors
>      example:
>                 .version_id = 1,
>                 .minimum_version_id = 1,
>                 .fields = (VMStateField[]) {
>                     VMSTATE_VBUFFER_UINT32(data, Fifo8, 1, NULL, 0, capacity),
>      uint32_t: present in 414 parsing errors
>      example:
>                                         uint32_t *bg, uint32_t *fg,                 \
>                                         VncPalette **palette) {                     \
>                     uint##bpp##_t *data;                                            \
>                     uint##bpp##_t c0, c1, ci;                                       \
>      env: present in 400 parsing errors
>      example:
>                 int v = float32_compare_quiet(a, b, &env->fp_status);
>                 set_br(env, v != float_relation_greater, br);
>             }
>      version_id: present in 308 parsing errors
>      example:
>                 .version_id = 1,
>                 .minimum_version_id = 1,
>                 .fields = (VMStateField[]) {
>                     VMSTATE_VBUFFER_UINT32(data, Fifo8, 1, NULL, 0, capacity),
>      x: present in 221 parsing errors
>      example:
>                 Int128 a = expand(x);
>                 Int128 r = int128_rshift(a, n);
>                 g_assert_cmpuint(r.lo, ==, l);
>                 g_assert_cmpuint(r.hi, ==, h);
>      a: present in 197 parsing errors
>      example:
>                 vmstate_load_state(loading, &vmstate_skipping, &obj, 2);
>                 g_assert(!qemu_file_get_error(loading));
>                 g_assert_cmpint(obj.a, ==, 10);
>                 g_assert_cmpint(obj.b, ==, 20);
>
> After:
>
>      -----------------------------------------------------------------------
>      maybe 10 most problematic tokens
>      -----------------------------------------------------------------------
>      NULL: present in 506 parsing errors
>      example:
>                 vs->dev = (QVirtioDevice *)dev;
>                 g_assert(dev != NULL);
>                 g_assert_cmphex(vs->dev->device_type, ==, QVIRTIO_SCSI_DEVICE_ID);
>
>      VMStateField: present in 502 parsing errors
>      example:
>                 .version_id = 1,
>                 .minimum_version_id = 1,
>                 .fields = (VMStateField[]) {
>                     VMSTATE_VBUFFER_UINT32(data, Fifo8, 1, NULL, 0, capacity),
>      fields: present in 502 parsing errors
>      example:
>                 .version_id = 1,
>                 .minimum_version_id = 1,
>                 .fields = (VMStateField[]) {
>                     VMSTATE_VBUFFER_UINT32(data, Fifo8, 1, NULL, 0, capacity),
>      minimum_version_id: present in 433 parsing errors
>      example:
>                 .version_id = 1,
>                 .minimum_version_id = 1,
>                 .fields = (VMStateField[]) {
>                     VMSTATE_VBUFFER_UINT32(data, Fifo8, 1, NULL, 0, capacity),
>      env: present in 376 parsing errors
>      example:
>                 int v = float32_compare_quiet(a, b, &env->fp_status);
>                 set_br(env, v != float_relation_greater, br);
>             }
>      version_id: present in 308 parsing errors
>      example:
>                 .version_id = 1,
>                 .minimum_version_id = 1,
>                 .fields = (VMStateField[]) {
>                     VMSTATE_VBUFFER_UINT32(data, Fifo8, 1, NULL, 0, capacity),
>      x: present in 221 parsing errors
>      example:
>                 Int128 a = expand(x);
>                 Int128 r = int128_rshift(a, n);
>                 g_assert_cmpuint(r.lo, ==, l);
>                 g_assert_cmpuint(r.hi, ==, h);
>      a: present in 197 parsing errors
>      example:
>                 vmstate_load_state(loading, &vmstate_skipping, &obj, 2);
>                 g_assert(!qemu_file_get_error(loading));
>                 g_assert_cmpint(obj.a, ==, 10);
>                 g_assert_cmpint(obj.b, ==, 20);
>      b: present in 164 parsing errors
>      example:
>                     uint64_t rl, rh;
>                     muls64(&rl, &rh, test_s_data[i].a, test_s_data[i].b);
>                     g_assert_cmpuint(rl, ==, test_s_data[i].rl);
>                     g_assert_cmpint(rh, ==, test_s_data[i].rh);
>      name: present in 150 parsing errors
>      example:
>                 g_assert(list != NULL);
>                 g_assert(QTAILQ_EMPTY(&list->head));
>                 g_assert_cmpstr(list->name, ==, "opts_list_01");
>
>      -----------------------------------------------------------------------
>
> Clearly visible is the effect of your cocci-macro-file.h's glue() .
>
>> ---
>>   scripts/cocci-macro-file.h | 119 +++++++++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 119 insertions(+)
>>   create mode 100644 scripts/cocci-macro-file.h
>>
>> diff --git a/scripts/cocci-macro-file.h b/scripts/cocci-macro-file.h
>> new file mode 100644
>> index 0000000..eceb4be
>> --- /dev/null
>> +++ b/scripts/cocci-macro-file.h
>> @@ -0,0 +1,119 @@
>> +/* Macro file for Coccinelle
>> + *
>> + * Copyright (C) 2015 Red Hat, Inc.
>> + *
>> + * Authors:
>> + *  Paolo Bonzini <pbonzini@redhat.com>
>> + *
>> + * This work is licensed under the terms of the GNU GPL, version 2 or, at your
>> + * option, any later version.  See the COPYING file in the top-level directory.
>> + */
>> +
>> +/* Coccinelle only does limited parsing of headers, and chokes on some idioms
>> + * defined in compiler.h and queue.h.  Macros that Coccinelle must know about
>> + * in order to parse .c files must be in a separate macro file---which is
>> + * exactly what you're staring at now.
>> + *
>> + * To use this file, add the "--macro-file scripts/cocci-macro-file.h" to the
>> + * Coccinelle command line.
>> + */
>> +
>> +/* From qemu/compiler.h */
>> +#define QEMU_GNUC_PREREQ(maj, min) 1
>> +#define QEMU_NORETURN __attribute__ ((__noreturn__))
>> +#define QEMU_WARN_UNUSED_RESULT __attribute__((warn_unused_result))
>> +#define QEMU_SENTINEL __attribute__((sentinel))
>> +#define QEMU_ARTIFICIAL __attribute__((always_inline, artificial))
>> +#define QEMU_PACKED __attribute__((gcc_struct, packed))
>> +
>> +#define cat(x,y) x ## y
>> +#define cat2(x,y) cat(x,y)
>
> Why not reuse glue()?
>
>> +#define QEMU_BUILD_BUG_ON(x) \
>> + typedef char cat2(qemu_build_bug_on__,__LINE__)[(x)?-1:1]
>> __attribute__((unused));
>> +
>> +#define GCC_FMT_ATTR(n, m) __attribute__((format(gnu_printf, n, m)))
>> +
>> +#define xglue(x, y) x ## y
>> +#define glue(x, y) xglue(x, y)
>> +#define stringify(s)	tostring(s)
>> +#define tostring(s)	#s
>> +
>> +#define typeof_field(type, field) typeof(((type *)0)->field)
>> +#define type_check(t1,t2) ((t1*)0 - (t2*)0)
>> +
> [...]
>
> Preferably with cat2() replaced by glue():
> Reviewed-by: Markus Armbruster <armbru@redhat.com>
>

Re: [Qemu-devel] [PATCH v2] add macro file for coccinelle

[John Snow]

On 09/07/2015 06:21 AM, Paolo Bonzini wrote:
> Coccinelle chokes on some idioms from compiler.h and queue.h.
> Extract those in a macro file, to be used with "--macro-file
> scripts/cocci-macro-file.h".
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  scripts/cocci-macro-file.h | 119 +++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 119 insertions(+)
>  create mode 100644 scripts/cocci-macro-file.h
> 
> diff --git a/scripts/cocci-macro-file.h b/scripts/cocci-macro-file.h
> new file mode 100644
> index 0000000..eceb4be
> --- /dev/null
> +++ b/scripts/cocci-macro-file.h
> @@ -0,0 +1,119 @@
> +/* Macro file for Coccinelle
> + *
> + * Copyright (C) 2015 Red Hat, Inc.
> + *
> + * Authors:
> + *  Paolo Bonzini <pbonzini@redhat.com>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or, at your
> + * option, any later version.  See the COPYING file in the top-level directory.
> + */
> +
> +/* Coccinelle only does limited parsing of headers, and chokes on some idioms
> + * defined in compiler.h and queue.h.  Macros that Coccinelle must know about
> + * in order to parse .c files must be in a separate macro file---which is
> + * exactly what you're staring at now.
> + *
> + * To use this file, add the "--macro-file scripts/cocci-macro-file.h" to the
> + * Coccinelle command line.
> + */
> +
> +/* From qemu/compiler.h */
> +#define QEMU_GNUC_PREREQ(maj, min) 1
> +#define QEMU_NORETURN __attribute__ ((__noreturn__))
> +#define QEMU_WARN_UNUSED_RESULT __attribute__((warn_unused_result))
> +#define QEMU_SENTINEL __attribute__((sentinel))
> +#define QEMU_ARTIFICIAL __attribute__((always_inline, artificial))
> +#define QEMU_PACKED __attribute__((gcc_struct, packed))
> +
> +#define cat(x,y) x ## y
> +#define cat2(x,y) cat(x,y)
> +#define QEMU_BUILD_BUG_ON(x) \
> +    typedef char cat2(qemu_build_bug_on__,__LINE__)[(x)?-1:1] __attribute__((unused));
> +
> +#define GCC_FMT_ATTR(n, m) __attribute__((format(gnu_printf, n, m)))
> +
> +#define xglue(x, y) x ## y
> +#define glue(x, y) xglue(x, y)
> +#define stringify(s)	tostring(s)
> +#define tostring(s)	#s
> +
> +#define typeof_field(type, field) typeof(((type *)0)->field)
> +#define type_check(t1,t2) ((t1*)0 - (t2*)0)
> +
> +/* From qemu/queue.h */
> +
> +#define QLIST_HEAD(name, type)                                          \
> +struct name {                                                           \
> +        struct type *lh_first;  /* first element */                     \
> +}
> +
> +#define QLIST_HEAD_INITIALIZER(head)                                    \
> +        { NULL }
> +
> +#define QLIST_ENTRY(type)                                               \
> +struct {                                                                \
> +        struct type *le_next;   /* next element */                      \
> +        struct type **le_prev;  /* address of previous next element */  \
> +}
> +
> +/*
> + * Singly-linked List definitions.
> + */
> +#define QSLIST_HEAD(name, type)                                          \
> +struct name {                                                           \
> +        struct type *slh_first; /* first element */                     \
> +}
> +
> +#define QSLIST_HEAD_INITIALIZER(head)                                    \
> +        { NULL }
> +
> +#define QSLIST_ENTRY(type)                                               \
> +struct {                                                                \
> +        struct type *sle_next;  /* next element */                      \
> +}
> +
> +/*
> + * Simple queue definitions.
> + */
> +#define QSIMPLEQ_HEAD(name, type)                                       \
> +struct name {                                                           \
> +    struct type *sqh_first;    /* first element */                      \
> +    struct type **sqh_last;    /* addr of last next element */          \
> +}
> +
> +#define QSIMPLEQ_HEAD_INITIALIZER(head)                                 \
> +    { NULL, &(head).sqh_first }
> +
> +#define QSIMPLEQ_ENTRY(type)                                            \
> +struct {                                                                \
> +    struct type *sqe_next;    /* next element */                        \
> +}
> +
> +/*
> + * Tail queue definitions.
> + */
> +#define Q_TAILQ_HEAD(name, type, qual)                                  \
> +struct name {                                                           \
> +        qual type *tqh_first;           /* first element */             \
> +        qual type *qual *tqh_last;      /* addr of last next element */ \
> +}
> +#define QTAILQ_HEAD(name, type)                                         \
> +struct name {                                                           \
> +        type *tqh_first;      /* first element */                       \
> +        type **tqh_last;      /* addr of last next element */           \
> +}
> +
> +#define QTAILQ_HEAD_INITIALIZER(head)                                   \
> +        { NULL, &(head).tqh_first }
> +
> +#define Q_TAILQ_ENTRY(type, qual)                                       \
> +struct {                                                                \
> +        qual type *tqe_next;            /* next element */              \
> +        qual type *qual *tqe_prev;      /* address of previous next element */\
> +}
> +#define QTAILQ_ENTRY(type)                                              \
> +struct {                                                                \
> +        type *tqe_next;       /* next element */                        \
> +        type **tqe_prev;      /* address of previous next element */    \
> +}
> 

Having not used Coccinelle yet, what happens if any of these functions
become desynchronized ?

Re: [Qemu-devel] [PATCH v2] add macro file for coccinelle

[Markus Armbruster]

John Snow <jsnow@redhat.com> writes:

> On 09/07/2015 06:21 AM, Paolo Bonzini wrote:
>> Coccinelle chokes on some idioms from compiler.h and queue.h.
>> Extract those in a macro file, to be used with "--macro-file
>> scripts/cocci-macro-file.h".
>> 
>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>> ---
>>  scripts/cocci-macro-file.h | 119
>> +++++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 119 insertions(+)
>>  create mode 100644 scripts/cocci-macro-file.h
>> 
>> diff --git a/scripts/cocci-macro-file.h b/scripts/cocci-macro-file.h
>> new file mode 100644
>> index 0000000..eceb4be
>> --- /dev/null
>> +++ b/scripts/cocci-macro-file.h
>> @@ -0,0 +1,119 @@
>> +/* Macro file for Coccinelle
>> + *
>> + * Copyright (C) 2015 Red Hat, Inc.
>> + *
>> + * Authors:
>> + *  Paolo Bonzini <pbonzini@redhat.com>
>> + *
>> + * This work is licensed under the terms of the GNU GPL, version 2
>> or, at your
>> + * option, any later version.  See the COPYING file in the
>> top-level directory.
>> + */
>> +
>> +/* Coccinelle only does limited parsing of headers, and chokes on some idioms
>> + * defined in compiler.h and queue.h.  Macros that Coccinelle must know about
>> + * in order to parse .c files must be in a separate macro file---which is
>> + * exactly what you're staring at now.
>> + *
>> + * To use this file, add the "--macro-file scripts/cocci-macro-file.h" to the
>> + * Coccinelle command line.
>> + */
>> +
>> +/* From qemu/compiler.h */
[Stuff cribbed from various headers...]
>
> Having not used Coccinelle yet, what happens if any of these functions
> become desynchronized ?

Coccinelle transforms C to C.  Problem: if you parse C the normal way
(first run preprocessor, then the parser proper), you have to unparse
and unpreprocess at the end to get a useful result.  Unparsing is easy,
but unpreprocessing is hard.

Instead, Coccinelle tries to parse *unpreprocessed* C.  Works most of
the time, because most uses of function-like macros can be treated as if
they were function calls, and most uses of object-like macros can be
treated as if they were values.

When it doesn't work, Coccinelle needs to resort to magic and / or skip
over some code it can't decipher.  The latter is undesirable, because if
the skipped code contains something we'd like to transform, we won't.

Part of the magic is treating "bad" macros specially.  --macro-file
helps with that part: macros defined there are "bad".  Sorry, I can't
really explain it, -EMAGIC.  /usr/share/coccinelle/standard.h is used by
default.

So what happens when the this file gets out of sync?  Worst case is
Coccinelle misses a pattern it could find if it was in sync.

Provided the macro file makes sense initially, a moderately bit-rotten
version is still almost certainly better than nothing.

Re: [Qemu-devel] [PATCH v2] add macro file for coccinelle

[Paolo Bonzini]

On 08/09/2015 20:35, Markus Armbruster wrote:
>> > Having not used Coccinelle yet, what happens if any of these functions
>> > become desynchronized ?
> Coccinelle transforms C to C.  Problem: if you parse C the normal way
> (first run preprocessor, then the parser proper), you have to unparse
> and unpreprocess at the end to get a useful result.  Unparsing is easy,
> but unpreprocessing is hard.
> 
> Instead, Coccinelle tries to parse *unpreprocessed* C.  Works most of
> the time, because most uses of function-like macros can be treated as if
> they were function calls, and most uses of object-like macros can be
> treated as if they were values.
> 
> When it doesn't work, Coccinelle needs to resort to magic and / or skip
> over some code it can't decipher.  The latter is undesirable, because if
> the skipped code contains something we'd like to transform, we won't.
> 
> Part of the magic is treating "bad" macros specially.  --macro-file
> helps with that part: macros defined there are "bad".  Sorry, I can't
> really explain it, -EMAGIC.  /usr/share/coccinelle/standard.h is used by
> default.
> 
> So what happens when the this file gets out of sync?  Worst case is
> Coccinelle misses a pattern it could find if it was in sync.
> 
> Provided the macro file makes sense initially, a moderately bit-rotten
> version is still almost certainly better than nothing.

Great answer.  I'll only add that exactly the same issue happens with
the Coverity model.

Paolo

Re: [Qemu-devel] [PATCH v2] add macro file for coccinelle

[John Snow]

On 09/09/2015 05:51 AM, Paolo Bonzini wrote:
> 
> 
> On 08/09/2015 20:35, Markus Armbruster wrote:
>>>> Having not used Coccinelle yet, what happens if any of these functions
>>>> become desynchronized ?
>> Coccinelle transforms C to C.  Problem: if you parse C the normal way
>> (first run preprocessor, then the parser proper), you have to unparse
>> and unpreprocess at the end to get a useful result.  Unparsing is easy,
>> but unpreprocessing is hard.
>>
>> Instead, Coccinelle tries to parse *unpreprocessed* C.  Works most of
>> the time, because most uses of function-like macros can be treated as if
>> they were function calls, and most uses of object-like macros can be
>> treated as if they were values.
>>
>> When it doesn't work, Coccinelle needs to resort to magic and / or skip
>> over some code it can't decipher.  The latter is undesirable, because if
>> the skipped code contains something we'd like to transform, we won't.
>>
>> Part of the magic is treating "bad" macros specially.  --macro-file
>> helps with that part: macros defined there are "bad".  Sorry, I can't
>> really explain it, -EMAGIC.  /usr/share/coccinelle/standard.h is used by
>> default.
>>
>> So what happens when the this file gets out of sync?  Worst case is
>> Coccinelle misses a pattern it could find if it was in sync.
>>
>> Provided the macro file makes sense initially, a moderately bit-rotten
>> version is still almost certainly better than nothing.
> 
> Great answer.  I'll only add that exactly the same issue happens with
> the Coverity model.
> 
> Paolo
> 

Great, thanks for the info :)

--js

Re: [Qemu-devel] [PATCH v2] add macro file for coccinelle

[Michael Tokarev]

07.09.2015 13:21, Paolo Bonzini wrote:
> Coccinelle chokes on some idioms from compiler.h and queue.h.
> Extract those in a macro file, to be used with "--macro-file
> scripts/cocci-macro-file.h".

Applied to -trivial, thank you!

/mjt