[patch] standard_session_servicedirs (was Re: OLPC and .service
files in a users home directory)
Havoc Pennington
hp at redhat.com
Wed Nov 1 08:17:22 PST 2006
Hi,
John (J5) Palmieri wrote:
> Ask and you shall receive, complete with a test case.
Thanks! comments inline
> _DBUS_ASSERT_ERROR_IS_CLEAR (error);
>
> - directory = _dbus_string_get_const_data (dir);
> + directory = _dbus_string_get_data (dir);
>
> if (stat (directory, &sb) < 0)
This change doesn't look right to me. "directory" is const. get_data()
will fail if used on a const DBusString too, which this could be.
> +/*
> + * Returns the standard directories for a session bus to look for service
> + * activation files
The doc comment needs "/**" to be picked up
> +
> + if (!_dbus_string_append (&path, xdg_data_home))
> + {
> + _dbus_string_free (&path);
> + goto oom;
> + }
> +
> + if (!_dbus_string_append (&path, DBUS_UNIX_STANDARD_SESSION_SERVICEDIR))
It would be good to use _dbus_concat_dir_and_file() so things work
whether or not there's a trailing "/"
(same for the other cases in this function)
> + cpath = _dbus_strdup ("/usr/share"DBUS_UNIX_STANDARD_SESSION_SERVICEDIR);
This should be --datadir from configure, rather than /usr/share
hardcoded, or at least --datadir from configure should also be included
> + cpath = _dbus_strdup ("/usr/local/share"DBUS_UNIX_STANDARD_SESSION_SERVICEDIR);
Does the xdg spec require this I guess?
> +#define DBUS_UNIX_STANDARD_SESSION_SERVICEDIR "/dbus-1/services"
> +
Could just put this in the .c file to avoid misuse
> /** An opaque string type */
> typedef struct DBusString DBusString;
>
> +/** avoid circular includes with DBusList */
> +struct DBusList;
Why not do this the same way as the DBusString above it (i.e. move the
typedef over from dbus-list.h)
Havoc
More information about the dbus
mailing list