patch for command line arguments in activation

[Rodrigo Moya]

On Sat, 2005-07-02 at 02:13 -0400, John (J5) Palmieri wrote:
> Ok, I spent some time looking at your code.  A couple of things:
> 
> - you need to include the dbus-shell.h headers where you use the
> functions.
> 
ok, done

> - you need to return 0 from your main of your test
> 
done also

> - the code does not handle OOM errors, it needs to:
> /* Now try to spawn the process */
>   if (!_dbus_shell_parse_argv (entry->exec, &argc, &argv))
>     {
>       _dbus_verbose ("Failed to parse command line: %s\n", entry->exec);
> 
>       /* this will assert every time because _dbus_shell_parse_argv 
>          does not return a DBusError.  If a FALSE return can only 
>          happen because OOM then you need to set the error to OOM
>       */      
>       _DBUS_ASSERT_ERROR_IS_SET (error);    
>       return FALSE;
>     }
ok, changed it to BUS_SET_OOM

> 
> - the tests failed with this:
> 
> check_segfault_service_no_auto_start:2701 received message interface
> "(unset)" member "(unset)" error name
> "org.freedesktop.DBus.Error.Spawn.ExecFailed" on 0x9141e58, expecting
> not this error
> segfault_service_no_auto_start failed during oom
> File "dispatch.c" line 3086 process 4401 should not have been reached:
> test failed
> /bin/sh: line 1:  4401 Aborted
> DBUS_TEST_DATA=../test/data DBUS_TEST_HOMEDIR=../dbus ${dir}$tst
> FAIL: bus-test
> ===================
> 1 of 1 tests failed
> ===================
> 
> A well placed printf loop over your parsed argv:
> 
> /* Now try to spawn the process */
>   if (!_dbus_shell_parse_argv (entry->exec, &argc, &argv))
>     {
>       _dbus_verbose ("Failed to parse command line: %s\n", entry->exec);
>       _DBUS_ASSERT_ERROR_IS_SET (error);
>       
>       return FALSE;
>     }
>   
>   {
>     int i;
>     for (i = 0; argv[i] != NULL; ++i)
>       printf ("element: %s\n", argv[i]);
>   }
> 
> reveled this:
> 
> element: /home/boston/johnp/devel-local/dbus/test/test-segfaul
> 
> should be:
> 
> element: /home/boston/johnp/devel-local/dbus/test/test-segfault
> 
weird, this indeed happens, on the same command line, after 70 or so
runs:

Running /opt/extra/src/dbus/test/test-segfault ...
Running /opt/extra/src/dbus/test/test-segfault ...
Running /opt/extra/src/dbus/test/test-segfault ...
...
Running /opt/extra/src/dbus/test/test-segfault ...
Running /opt/extra/src/dbus/test/test-segfault ...
Running /opt/extra/src/dbus/test/test-segfaul ...

> You have an off by one error or memory corruption in your parsing code.
> 
seems from my debugging, that the program might be running out of
memory? Running through valgrind shows some of these:

==6782==    at 0x1B9052B6: malloc
(in /usr/lib/valgrind/vgpreload_memcheck.so)
==6782==    by 0x809174A: dbus_malloc (dbus-memory.c:469)
==6782==    by 0x809ECAD: _dbus_shell_parse_argv (dbus-shell.c:582)
==6782==    by 0x804B768: bus_activation_activate_service
(activation.c:1525)
==6782==    by 0x805B84D: bus_driver_handle_activate_service
(driver.c:620)
==6782==    by 0x805C104: bus_driver_handle_message (driver.c:1308)
==6782==    by 0x805A2EE: bus_dispatch_message_filter (dispatch.c:239)
==6782==    by 0x806AF94: dbus_connection_dispatch
(dbus-connection.c:3558)
==6782==    by 0x809DD6A: _dbus_loop_dispatch (dbus-mainloop.c:482)
==6782==    by 0x809E210: _dbus_loop_iterate (dbus-mainloop.c:848)
==6782==    by 0x8061819: bus_test_run_everything (test.c:287)
==6782==    by 0x8059262: check_segfault_service_no_auto_start
(dispatch.c:2655)

Which I can't find where it's leaking here, since I am calling
dbus_free_string_array right after using the argv returned by
_dbus_shell_parse_argv. Any idea what I am missing?

and lots of these:

==6782== 12 bytes in 1 blocks are definitely lost in loss record 1 of 10
==6782==    at 0x1B905C05: calloc
(in /usr/lib/valgrind/vgpreload_memcheck.so)
==6782==    by 0x809169C: dbus_malloc0 (dbus-memory.c:519)
==6782==    by 0x805368C: bus_transaction_new (connection.c:1877)
==6782==    by 0x805A072: bus_dispatch_message_filter (dispatch.c:201)
==6782==    by 0x806AF94: dbus_connection_dispatch
(dbus-connection.c:3558)
==6782==    by 0x809DD6A: _dbus_loop_dispatch (dbus-mainloop.c:482)
==6782==    by 0x809E210: _dbus_loop_iterate (dbus-mainloop.c:848)
==6782==    by 0x8061819: bus_test_run_everything (test.c:287)
==6782==    by 0x8059262: check_segfault_service_no_auto_start
(dispatch.c:2655)
==6782==    by 0x8056723: check_oom_check2_func (dispatch.c:3056)
==6782==    by 0x808A134: _dbus_test_oom_handling (dbus-internals.c:519)
==6782==    by 0x80574D3: check2_try_iterations (dispatch.c:3080)

> You need to expand your test case to not just count the number of
> elements returned but compare them to expected strings.  Also I noticed
> it got things right during the OOM tests which means you need to run the
> tests multiple times in succession, perhaps with random data.
> 
I've rewritten the shell-test program to make the checks itself, find it
attached with the rest of the patch. This test program works great.
-- 
Rodrigo Moya <rodrigo at gnome-db.org>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: dbus-activation.diff
Type: text/x-patch
Size: 4360 bytes
Desc: not available
Url : http://lists.freedesktop.org/archives/dbus/attachments/20050702/d4f6a6a6/dbus-activation-0001.bin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: dbus-shell.c
Type: text/x-csrc
Size: 17754 bytes
Desc: not available
Url : http://lists.freedesktop.org/archives/dbus/attachments/20050702/d4f6a6a6/dbus-shell-0001.c
-------------- next part --------------
A non-text attachment was scrubbed...
Name: dbus-shell.h
Type: text/x-chdr
Size: 1375 bytes
Desc: not available
Url : http://lists.freedesktop.org/archives/dbus/attachments/20050702/d4f6a6a6/dbus-shell-0001.h
-------------- next part --------------
A non-text attachment was scrubbed...
Name: shell-test.c
Type: text/x-csrc
Size: 3218 bytes
Desc: not available
Url : http://lists.freedesktop.org/archives/dbus/attachments/20050702/d4f6a6a6/shell-test-0001.c