[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