patch for command line arguments in activation

[John (J5) Palmieri]

On Sat, 2005-07-02 at 20:28 +0200, Rodrigo Moya wrote:
> 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

This is fine as long as the only error that can happen is OOM.  You want
to return a dbus error if there are any other conditions.

> 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:

The tests run through a series of out of memory check where it sets dbus
malloc functions to return OOM to see if dbus can recover from these
situations.  You are going to get OOM in your test, your code just need
to handle and propagate the error correctly.  This means that anytime
you do an operation that may run out of memory (appending to string or
list) you need to check the return value and handle the OOM which
usually means just returning OOM from your function.

I also noticed this:

/*you need to return a dbus_bool_t FALSE for OOM*/
static void
delimit_token (DBusString **token,
               DBusList **retval)
{
  if (*token == NULL)
    return;

  /* don't do the strdup inline.  Use a temporary 
     var and check for OOM*/
  _dbus_list_append (retval, _dbus_strdup (_dbus_string_get_data
(*token)));

  /* This looks like a double free to me. 
     Also why are you freeing here?
     Seems to me this function doesn't own *token
  */
  _dbus_string_free (*token);
  dbus_free (*token);

  *token = NULL;
}

-- 
John (J5) Palmieri <johnp at redhat.com>