Re: [Qemu-devel] [PATCH v4 0/3] qapi: fix coding style in generated code

[Amos Kong]

On Fri, May 16, 2014 at 10:19:02AM -0400, Luiz Capitulino wrote:
> On Thu,  8 May 2014 09:14:37 +0800
> Amos Kong <address@hidden> wrote:
> 
> > Not a serious issue, but it's helpful if we can fix it.
> > 
> > V2: split change of scripts/qapi-visit.py to a split patch,
> >     eat space by using a special char as Markus suggested
> > V3: update commitlog, update special string, fix of adding
> >     const replace string by pattern
> > V4: fix pattern to cleanup special string (Paolo)
> > 
> > Amos Kong (3):
> >   qapi: fix coding style in parameters list
> >   qapi: add const prefix to 'char *' insider c_type()
> >   qapi: Suppress unwanted space between type and identifier
> > 
> >  scripts/qapi-commands.py |  4 +---
> >  scripts/qapi-visit.py    | 20 ++++++++++----------
> >  scripts/qapi.py          | 18 ++++++++++++------
> >  3 files changed, 23 insertions(+), 19 deletions(-)
> 
> Amos, it seems that this series breaks test-qmp-commands (trace below).

Thanks for the catch.
 
> I'm not sure what is causing this and at first I thought your series was
> only making an existing bug visible, but I took a quick look and it seems
> that your last patch changes the code generator to drop the initialization
> of generated types pointers in (generated) qmp commands.
> 
> Can you please investigate this? And also, please, carry Reviewed-bys
> of unmodified patches and address Eric's comments if you respin.

I found the root reason.

We added a suffix in c_type() for pointer types, and clean it
in mcgen(). But there is a place that we directly check return
value of c_type().

I will respin.

A quick fix:

diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py
index 8f8a258..980573e 100644
--- a/scripts/qapi-commands.py
+++ b/scripts/qapi-commands.py
@@ -90,7 +92,7 @@ def gen_visitor_input_vars_decl(args):
 bool has_%(argname)s = false;
 ''',
                          argname=c_var(argname))
-        if c_type(argtype).endswith("*"):
+        if c_type(argtype).endswith("*\033EATSPACE."):
             ret += mcgen('''
 %(argtype)s %(argname)s = NULL;
 ''',

Thanks, Amos
 
> $ make check
> [...]
> GTESTER tests/test-qmp-commands
> *** Error in `tests/test-qmp-commands': free(): invalid pointer: 
> 0x00007f8a2fef3030 ***
> ======= Backtrace: =========
> /lib64/libc.so.6(+0x3926875cff)[0x7f8a2d421cff]
> /lib64/libc.so.6(+0x392687cff8)[0x7f8a2d428ff8]
> /lib64/libglib-2.0.so.0(g_free+0xf)[0x7f8a2e616f7f]
> tests/test-qmp-commands(+0x2b267)[0x7f8a2ef4b267]
> tests/test-qmp-commands(+0x2a3fb)[0x7f8a2ef4a3fb]
> tests/test-qmp-commands(+0x262ca)[0x7f8a2ef462ca]
> tests/test-qmp-commands(+0x2633b)[0x7f8a2ef4633b]
> tests/test-qmp-commands(+0x26494)[0x7f8a2ef46494]
> tests/test-qmp-commands(+0x29f73)[0x7f8a2ef49f73]
> tests/test-qmp-commands(+0x2d2ac)[0x7f8a2ef4d2ac]
> tests/test-qmp-commands(+0x2d3de)[0x7f8a2ef4d3de]
> tests/test-qmp-commands(+0x291ee)[0x7f8a2ef491ee]
> /lib64/libglib-2.0.so.0(+0x392946d5e1)[0x7f8a2e6355e1]
> /lib64/libglib-2.0.so.0(+0x392946d7a6)[0x7f8a2e6357a6]
> /lib64/libglib-2.0.so.0(g_test_run_suite+0x17b)[0x7f8a2e635b1b]
> tests/test-qmp-commands(main+0x9a)[0x7f8a2ef49bac]
> /lib64/libc.so.6(__libc_start_main+0xf5)[0x7f8a2d3cdd65]
> tests/test-qmp-commands(+0x4499)[0x7f8a2ef24499]
> ======= Memory map: ========
> 7f8a2d3ac000-7f8a2d560000 r-xp 00000000 fd:00 551385                     
> /usr/lib64/libc-2.18.so
> 7f8a2d560000-7f8a2d760000 ---p 001b4000 fd:00 551385                     
> /usr/lib64/libc-2.18.so
> 7f8a2d760000-7f8a2d764000 r--p 001b4000 fd:00 551385                     
> /usr/lib64/libc-2.18.so
> 7f8a2d764000-7f8a2d766000 rw-p 001b8000 fd:00 551385                     
> /usr/lib64/libc-2.18.so
> 7f8a2d766000-7f8a2d76b000 rw-p 00000000 00:00 0 
> 7f8a2d76b000-7f8a2d783000 r-xp 00000000 fd:00 530647                     
> /usr/lib64/libpthread-2.18.so
> 7f8a2d783000-7f8a2d982000 ---p 00018000 fd:00 530647                     
> /usr/lib64/libpthread-2.18.so
> 7f8a2d982000-7f8a2d983000 r--p 00017000 fd:00 530647                     
> /usr/lib64/libpthread-2.18.so
> 7f8a2d983000-7f8a2d984000 rw-p 00018000 fd:00 530647                     
> /usr/lib64/libpthread-2.18.so
> 7f8a2d984000-7f8a2d988000 rw-p 00000000 00:00 0 
> 7f8a2d988000-7f8a2d99d000 r-xp 00000000 fd:00 551415                     
> /usr/lib64/libgcc_s-4.8.2-20131212.so.1
> 7f8a2d99d000-7f8a2db9c000 ---p 00015000 fd:00 551415                     
> /usr/lib64/libgcc_s-4.8.2-20131212.so.1
> 7f8a2db9c000-7f8a2db9d000 r--p 00014000 fd:00 551415                     
> /usr/lib64/libgcc_s-4.8.2-20131212.so.1
> 7f8a2db9d000-7f8a2db9e000 rw-p 00015000 fd:00 551415                     
> /usr/lib64/libgcc_s-4.8.2-20131212.so.1
> 7f8a2db9e000-7f8a2dca3000 r-xp 00000000 fd:00 569400                     
> /usr/lib64/libm-2.18.so
> 7f8a2dca3000-7f8a2dea3000 ---p 00105000 fd:00 569400                     
> /usr/lib64/libm-2.18.so
> 7f8a2dea3000-7f8a2dea4000 r--p 00105000 fd:00 569400                     
> /usr/lib64/libm-2.18.so
> 7f8a2dea4000-7f8a2dea5000 rw-p 00106000 fd:00 569400                     
> /usr/lib64/libm-2.18.so
> 7f8a2dea5000-7f8a2df8e000 r-xp 00000000 fd:00 569411                     
> /usr/lib64/libstdc++.so.6.0.19
> 7f8a2df8e000-7f8a2e18e000 ---p 000e9000 fd:00 569411                     
> /usr/lib64/libstdc++.so.6.0.19
> 7f8a2e18e000-7f8a2e196000 r--p 000e9000 fd:00 569411                     
> /usr/lib64/libstdc++.so.6.0.19
> 7f8a2e196000-7f8a2e198000 rw-p 000f1000 fd:00 569411                     
> /usr/lib64/libstdc++.so.6.0.19
> 7f8a2e198000-7f8a2e1ad000 rw-p 00000000 00:00 0 
> 7f8a2e1ad000-7f8a2e1b1000 r-xp 00000000 fd:00 535852                     
> /usr/lib64/libuuid.so.1.3.0
> 7f8a2e1b1000-7f8a2e3b0000 ---p 00004000 fd:00 535852                     
> /usr/lib64/libuuid.so.1.3.0
> 7f8a2e3b0000-7f8a2e3b1000 r--p 00003000 fd:00 535852                     
> /usr/lib64/libuuid.so.1.3.0
> 7f8a2e3b1000-7f8a2e3b2000 rw-p 00004000 fd:00 535852                     
> /usr/lib64/libuuid.so.1.3.0
> 7f8a2e3b2000-7f8a2e3c7000 r-xp 00000000 fd:00 551391                     
> /usr/lib64/libz.so.1.2.8
> 7f8a2e3c7000-7f8a2e5c6000 ---p 00015000 fd:00 551391                     
> /usr/lib64/libz.so.1.2.8
> 7f8a2e5c6000-7f8a2e5c7000 r--p 00014000 fd:00 551391                     
> /usr/lib64/libz.so.1.2.8
> 7f8a2e5c7000-7f8a2e5c8000 rw-p 00015000 fd:00 551391                     
> /usr/lib64/libz.so.1.2.8
> 7f8a2e5c8000-7f8a2e6f1000 r-xp 00000000 fd:00 535571                     
> /usr/lib64/libglib-2.0.so.0.3800.2
> 7f8a2e6f1000-7f8a2e8f1000 ---p 00129000 fd:00 535571                     
> /usr/lib64/libglib-2.0.so.0.3800.2
> 7f8a2e8f1000-7f8a2e8f2000 r--p 00129000 fd:00 535571                     
> /usr/lib64/libglib-2.0.so.0.3800.2
> 7f8a2e8f2000-7f8a2e8f3000 rw-p 0012a000 fd:00 535571                     
> /usr/lib64/libglib-2.0.so.0.3800.2
> 7f8a2e8f3000-7f8a2e8f4000 rw-p 00000000 00:00 0 
> 7f8a2e8f4000-7f8a2e8f5000 r-xp 00000000 fd:00 551438                     
> /usr/lib64/libgthread-2.0.so.0.3800.2
> 7f8a2e8f5000-7f8a2eaf4000 ---p 00001000 fd:00 551438                     
> /usr/lib64/libgthread-2.0.so.0.3800.2
> 7f8a2eaf4000-7f8a2eaf5000 r--p 00000000 fd:00 551438                     
> /usr/lib64/libgthread-2.0.so.0.3800.2
> 7f8a2eaf5000-7f8a2eaf6000 rw-p 00001000 fd:00 551438                     
> /usr/lib64/libgthread-2.0.so.0.3800.2
> 7f8a2eaf6000-7f8a2eafd000 r-xp 00000000 fd:00 531444                     
> /usr/lib64/librt-2.18.so
> 7f8a2eafd000-7f8a2ecfc000 ---p 00007000 fd:00 531444                     
> /usr/lib64/librt-2.18.so
> 7f8a2ecfc000-7f8a2ecfd000 r--p 00006000 fd:00 531444                     
> /usr/lib64/librt-2.18.so
> 7f8a2ecfd000-7f8a2ecfe000 rw-p 00007000 fd:00 531444                     
> /usr/lib64/librt-2.18.so
> 7f8a2ecfe000-7f8a2ed1e000 r-xp 00000000 fd:00 551384                     
> /usr/lib64/ld-2.18.so
> 7f8a2eefa000-7f8a2ef02000 rw-p 00000000 00:00 0 
> 7f8a2ef1b000-7f8a2ef1d000 rw-p 00000000 00:00 0 
> 7f8a2ef1d000-7f8a2ef1e000 r--p 0001f000 fd:00 551384                     
> /usr/lib64/ld-2.18.so
> 7f8a2ef1e000-7f8a2ef1f000 rw-p 00020000 fd:00 551384                     
> /usr/lib64/ld-2.18.so
> 7f8a2ef1f000-7f8a2ef20000 rw-p 00000000 00:00 0 
> 7f8a2ef20000-7f8a2ef65000 r-xp 00000000 fd:04 282340                     
> /home/lcapitulino/work/src/upstream/qmp-unstable/build/tests/test-qmp-commands
> 7f8a2f165000-7f8a2f166000 r--p 00045000 fd:04 282340                     
> /home/lcapitulino/work/src/upstream/qmp-unstable/build/tests/test-qmp-commands
> 7f8a2f166000-7f8a2f167000 rw-p 00046000 fd:04 282340                     
> /home/lcapitulino/work/src/upstream/qmp-unstable/build/tests/test-qmp-commands
> 7f8a2fef0000-7f8a2ff11000 rw-p 00000000 00:00 0                          
> [heap]
> 7fffc9896000-7fffc98b8000 rw-p 00000000 00:00 0                          
> [stack]
> 7fffc98cd000-7fffc98cf000 r-xp 00000000 00:00 0                          
> [vdso]
> ffffffffff600000-ffffffffff601000 r-xp 00000000 00:00 0                  
> [vsyscall]
> GTester: last random seed: R02S53adc33aac3bcdb0168c5aa5f74a577d
> make: *** [check-tests/test-qmp-commands] Error 1
> 

-- 
                        Amos.