patch for command line arguments in activation

[John (J5) Palmieri]

On Sat, 2005-07-02 at 22:49 +0200, Rodrigo Moya wrote:
> On Sat, 2005-07-02 at 15:05 -0400, John (J5) Palmieri wrote:
> 
> > > 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.
> > 
> hmm, why doesn't dbus' memory functions deal with it, like glib does?

The point is to make sure all the code is propagating OOM errors.  The
memory functions can't handle OOM because it has no context to do so.
D-Bus will actually not die on OOM but wait until there is resources
available (well it could die but it tries its hardest not to).
It is fairly easy to support.  Just make sure any function that can fail
because of OOM check for and then somehow propagates the error (either
by returning FALSE or setting a DBusError).

> > 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;
> > }
> > 
> no, it's not a double free AFAICS. The caller expects delimit_token to
> add the string in 'token' to 'retval', and free it.

I would expect it to be freed in the same context it was initialized in.
What happens if a const string was passed in?  It may be an internal
function but still...

Other than that you are using DBusString wrong.  In order to be a bit
more secure and not expose pointers where it can be avoided dbus strings
are static structs containing a pointer to the real data within them.
Here is the lifecycle of a DBusString:

DBusString my_string;

dbus_string_init (&my_string);

dbus_string_append (&my_string, "append");

dbus_string_free (&my_string);

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