[patch] standard_session_servicedirs (was Re: OLPC and .service
files in a users home directory)
uwesmail2005-lkml at yahoo.de
uwesmail2005-lkml at yahoo.de
Thu Nov 2 04:53:12 PST 2006
--- "John (J5) Palmieri" <johnp at redhat.com> schrieb:
> On Wed, 2006-11-01 at 11:17 -0500, Havoc Pennington wrote:
> > 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.
>
> Stray edit that made it into the diff. This has been removed.
>
> > > +/*
> > > + * Returns the standard directories for a session bus to look
> for service
> > > + * activation files
> >
> > The doc comment needs "/**" to be picked up
>
> Fixed.
>
> > > +
> > > + 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)
>
> Fixed
>
> > > + 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
>
> I added DBUS_DATADIR to the list separate from the XDG list.
>
> > > + cpath = _dbus_strdup
> ("/usr/local/share"DBUS_UNIX_STANDARD_SESSION_SERVICEDIR);
> >
> > Does the xdg spec require this I guess?
>
> Yes it does.
>
> > > +#define DBUS_UNIX_STANDARD_SESSION_SERVICEDIR "/dbus-1/services"
> > > +
> >
> > Could just put this in the .c file to avoid misuse
>
> Fixed.
>
> > > /** 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)
>
> Fixed.
>
>
> I also took Thiago's suggestion from irc to simplify the code by
> creating a colon delimited list and then parsing this with the same
> code
> I had for parsing XDG_DATA_DIRS. This turned out to make the code
> much
> more readable.
>
> --
> John (J5) Palmieri <johnp at redhat.com>
> > Index: configure.in
> ===================================================================
> RCS file: /cvs/dbus/dbus/configure.in,v
> retrieving revision 1.194
> diff -u -p -r1.194 configure.in
> --- configure.in 26 Oct 2006 18:06:07 -0000 1.194
> +++ configure.in 1 Nov 2006 20:32:55 -0000
> @@ -1118,6 +1118,11 @@ fi
> AC_SUBST(DBUS_USER)
> AC_DEFINE_UNQUOTED(DBUS_USER,"$DBUS_USER", [User for running the
> system BUS daemon])
>
> +#### Direcotry to install data files into
> +DBUS_DATADIR=$EXPANDED_DATADIR
> +AC_SUBST(DBUS_DATADIR)
> +AC_DEFINE_UNQUOTED(DBUS_DATADIR,"$DBUS_DATADIR", [Directory for
> installing DBUS data files])
> +
> #### Directory to install dbus-daemon
> if test -z "$with_dbus_daemondir" ; then
> DBUS_DAEMONDIR=$EXPANDED_BINDIR
> Index: bus/config-parser.c
> ===================================================================
> RCS file: /cvs/dbus/dbus/bus/config-parser.c,v
> retrieving revision 1.44
> diff -u -p -r1.44 config-parser.c
> --- bus/config-parser.c 27 Oct 2006 18:30:22 -0000 1.44
> +++ bus/config-parser.c 1 Nov 2006 20:32:55 -0000
> @@ -47,7 +47,8 @@ typedef enum
> ELEMENT_INCLUDEDIR,
> ELEMENT_TYPE,
> ELEMENT_SELINUX,
> - ELEMENT_ASSOCIATE
> + ELEMENT_ASSOCIATE,
> + ELEMENT_STANDARD_SESSION_SERVICEDIRS
> } ElementType;
>
> typedef enum
> @@ -161,6 +162,8 @@ element_type_to_name (ElementType type)
> return "fork";
> case ELEMENT_PIDFILE:
> return "pidfile";
> + case ELEMENT_STANDARD_SESSION_SERVICEDIRS:
> + return "standard_session_servicedirs";
> case ELEMENT_SERVICEDIR:
> return "servicedir";
> case ELEMENT_INCLUDEDIR:
> @@ -800,6 +803,19 @@ start_busconfig_child (BusConfigParser
>
> return TRUE;
> }
> + else if (strcmp (element_name, "standard_session_servicedirs") ==
> 0)
Why not use element_type_to_name(ELEMENT_STANDARD_SESSION_SERVICEDIRS)
here? It would make maintaining (renaming the XML-Element) easier.
> + {
> + if (!check_no_attributes (parser,
> "standard_session_servicedirs", attribute_names, attribute_values,
> error))
> + return FALSE;
> +
> + if (push_element (parser,
> ELEMENT_STANDARD_SESSION_SERVICEDIRS) == NULL)
> + {
> + BUS_SET_OOM (error);
> + return FALSE;
> + }
> +
> + return TRUE;
> + }
> else if (strcmp (element_name, "servicedir") == 0)
> {
> if (!check_no_attributes (parser, "servicedir",
> attribute_names, attribute_values, error))
> @@ -1927,6 +1943,7 @@ bus_config_parser_end_element (BusConfig
> case ELEMENT_FORK:
> case ELEMENT_SELINUX:
> case ELEMENT_ASSOCIATE:
> + case ELEMENT_STANDARD_SESSION_SERVICEDIRS:
> break;
> }
>
> @@ -2335,7 +2352,18 @@ bus_config_parser_content (BusConfigPars
> }
> }
> break;
> + case ELEMENT_STANDARD_SESSION_SERVICEDIRS:
> + {
> + DBusList *link;
> + DBusList *dirs;
> + dirs = NULL;
>
> + if (!_dbus_get_standard_session_servicedirs (&dirs))
> + goto nomem;
> +
> + while ((link = _dbus_list_pop_first_link (&dirs)))
> + service_dirs_append_link_unique_or_free
> (&parser->service_dirs, link);
> + }
> case ELEMENT_SERVICEDIR:
> {
> char *s;
> @@ -3032,6 +3060,91 @@ process_test_equiv_subdir (const DBusStr
> return retval;
>
> }
> +
> +static const char *test_service_dir_matches[] =
> + {
> + "/testhome/foo/.testlocal/testshare/dbus-1/services",
> + DBUS_DATADIR"/dbus-1/services",
> + "/testusr/testshare/dbus-1/services",
> + "/testuser/testlocal/testshare/dbus-1/services",
> + NULL
> + };
> +
> +static dbus_bool_t
> +test_default_session_servicedirs (void)
> +{
> + DBusList *dirs;
> + DBusList *link;
> + int i;
> +
> + dirs = NULL;
> +
> + printf ("Testing retriving the default session service
> directories\n");
> + if (!_dbus_get_standard_session_servicedirs (&dirs))
> + _dbus_assert_not_reached ("couldn't get stardard dirs");
> +
> + /* make sure our defaults end with share/dbus-1/service */
> + while ((link = _dbus_list_pop_first_link (&dirs)))
> + {
> + DBusString path;
> +
> + printf (" default service dir: %s\n", (char *)link->data);
> + _dbus_string_init_const (&path, (char *)link->data);
> + if (!_dbus_string_ends_with_c_str (&path,
> "share/dbus-1/services"))
> + {
> + printf ("error with default session service
> directories\n");
> + return FALSE;
> + }
> +
> + dbus_free (link->data);
> + _dbus_list_free_link (link);
> + }
> +
> + if (!_dbus_setenv ("XDG_DATA_HOME",
> "/testhome/foo/.testlocal/testshare"))
> + _dbus_assert_not_reached ("couldn't setenv XDG_DATA_HOME");
> +
> + if (!_dbus_setenv ("XDG_DATA_DIRS",
> "/testusr/testshare:/testuser/testlocal/testshare"))
> + _dbus_assert_not_reached ("couldn't setenv XDG_DATA_DIRS");
> +
> + if (!_dbus_get_standard_session_servicedirs (&dirs))
> + _dbus_assert_not_reached ("couldn't get stardard dirs");
> +
> + /* make sure we read and parse the env variable correctly */
> + i = 0;
> + while ((link = _dbus_list_pop_first_link (&dirs)))
> + {
> + printf (" test service dir: %s\n", (char *)link->data);
> + if (test_service_dir_matches[i] == NULL)
> + {
> + printf ("more directories parsed than in match set\n");
> + return FALSE;
> + }
> +
> + if (strcmp (test_service_dir_matches[i],
> + (char *)link->data) != 0)
> + {
> + printf ("%s directory does not match %s in the match
> set\n",
> + (char *)link->data,
> + test_service_dir_matches[i]);
> + return FALSE;
> + }
> +
> + ++i;
> +
> + dbus_free (link->data);
> + _dbus_list_free_link (link);
> + }
> +
> + if (test_service_dir_matches[i] != NULL)
> + {
> + printf ("extra data %s in the match set was not matched\n",
> + test_service_dir_matches[i]);
> +
> + return FALSE;
> + }
> +
> + return TRUE;
> +}
>
> dbus_bool_t
> bus_config_parser_test (const DBusString *test_data_dir)
> @@ -3043,6 +3156,9 @@ bus_config_parser_test (const DBusString
> return TRUE;
> }
>
> + if (!test_default_session_servicedirs())
> + return FALSE;
> +
> if (!process_test_valid_subdir (test_data_dir,
> "valid-config-files", VALID))
> return FALSE;
>
> Index: bus/session.conf.in
> ===================================================================
> RCS file: /cvs/dbus/dbus/bus/session.conf.in,v
> retrieving revision 1.10
> diff -u -p -r1.10 session.conf.in
> --- bus/session.conf.in 3 Aug 2006 20:34:36 -0000 1.10
> +++ bus/session.conf.in 1 Nov 2006 20:32:55 -0000
> @@ -10,7 +10,7 @@
>
> <listen>unix:tmpdir=@DBUS_SESSION_SOCKET_DIR@</listen>
>
> - <servicedir>@EXPANDED_DATADIR@/dbus-1/services</servicedir>
> + <standard_session_servicedirs />
>
> <policy context="default">
> <!-- Allow everything to be sent -->
> Index: dbus/dbus-list.h
> ===================================================================
> RCS file: /cvs/dbus/dbus/dbus/dbus-list.h,v
> retrieving revision 1.15
> diff -u -p -r1.15 dbus-list.h
> --- dbus/dbus-list.h 3 Aug 2006 20:34:36 -0000 1.15
> +++ dbus/dbus-list.h 1 Nov 2006 20:32:55 -0000
> @@ -27,11 +27,10 @@
> #include <dbus/dbus-internals.h>
> #include <dbus/dbus-memory.h>
> #include <dbus/dbus-types.h>
> +#include <dbus/dbus-sysdeps.h>
>
> DBUS_BEGIN_DECLS
>
> -typedef struct DBusList DBusList;
> -
> struct DBusList
> {
> DBusList *prev; /**< Previous list node. */
> Index: dbus/dbus-sysdeps-unix.c
> ===================================================================
> RCS file: /cvs/dbus/dbus/dbus/dbus-sysdeps-unix.c,v
> retrieving revision 1.15
> diff -u -p -r1.15 dbus-sysdeps-unix.c
> --- dbus/dbus-sysdeps-unix.c 26 Oct 2006 19:01:10 -0000 1.15
> +++ dbus/dbus-sysdeps-unix.c 1 Nov 2006 20:32:55 -0000
> @@ -29,6 +29,8 @@
> #include "dbus-protocol.h"
> #include "dbus-transport.h"
> #include "dbus-string.h"
> +#include "dbus-userdb.h"
> +#include "dbus-list.h"
> #include <sys/types.h>
> #include <stdlib.h>
> #include <string.h>
> @@ -2528,4 +2530,213 @@ _dbus_read_local_machine_uuid (DBusGUID
> return _dbus_read_uuid_file (&filename, machine_id,
> create_if_not_found, error);
> }
>
> +#define DBUS_UNIX_STANDARD_SESSION_SERVICEDIR "/dbus-1/services"
> +
> +static dbus_bool_t
> +split_paths_and_append (DBusString *dirs,
> + const char *suffix,
> + DBusList **dir_list)
> +{
> + /* split on colon (:) */
> + int start;
> + int i;
> + int len;
> + char *cpath;
> + const DBusString file_suffix;
> +
> + start = 0;
> + i = 0;
> +
> + _dbus_string_init_const (&file_suffix, suffix);
> +
> + len = _dbus_string_get_length (dirs);
> +
> + while (_dbus_string_find (dirs, start, ":", &i))
> + {
> + DBusString path;
> +
> + if (!_dbus_string_init (&path))
> + goto oom;
> +
> + if (!_dbus_string_copy_len (dirs,
> + start,
> + i - start,
> + &path,
> + 0))
> + {
> + _dbus_string_free (&path);
> + goto oom;
> + }
> +
> + _dbus_string_chop_white (&path);
> +
> + /* check for an empty path */
> + if (_dbus_string_get_length (&path) == 0)
> + {
> + _dbus_string_free (&path);
> + continue;
> + }
> +
> + if (!_dbus_concat_dir_and_file (&path,
> + &file_suffix))
> + {
> + _dbus_string_free (&path);
> + goto oom;
> + }
> +
> + if (!_dbus_string_copy_data(&path, &cpath))
> + {
> + _dbus_string_free (&path);
> + goto oom;
> + }
> +
> + if (!_dbus_list_append (dir_list, cpath))
> + {
> + _dbus_string_free (&path);
> + dbus_free (cpath);
> + goto oom;
> + }
> +
> + _dbus_string_free (&path);
> + start = i + 1;
> + }
> +
> + if (start != len)
> + {
> + DBusString path;
> +
> + if (!_dbus_string_init (&path))
> + goto oom;
> +
> + if (!_dbus_string_copy_len (dirs,
> + start,
> + len - start,
> + &path,
> + 0))
> + {
> + _dbus_string_free (&path);
> + goto oom;
> + }
> +
> + if (!_dbus_concat_dir_and_file (&path,
> + &file_suffix))
> + {
> + _dbus_string_free (&path);
> + goto oom;
> + }
> +
> + if (!_dbus_string_copy_data(&path, &cpath))
> + {
> + _dbus_string_free (&path);
> + goto oom;
> + }
> +
> + if (!_dbus_list_append (dir_list, cpath))
> + {
> + _dbus_string_free (&path);
> + dbus_free (cpath);
> + goto oom;
> + }
> +
> + _dbus_string_free (&path);
> + }
> +
> + return TRUE;
> +
> + oom:
> + _dbus_list_foreach (dir_list, (DBusForeachFunction)dbus_free,
> NULL);
> + _dbus_list_clear (dir_list);
> + return FALSE;
> +}
> +
> +/**
> + * Returns the standard directories for a session bus to look for
> service
> + * activation files
> + *
> + * On UNIX this should be the standard xdg freedesktop.org data
> directories:
> + *
> + * XDG_DATA_HOME=${XDG_DATA_HOME-$HOME}/.local/share
> + * XDG_DATA_DIRS=${XDG_DATA_DIRS-/usr/share:/usr/local/share}
> + *
> + * and
> + *
> + * DBUS_DATADIR
> + *
> + * @param dirs the directory list we are returning
> + * @returns #FALSE on OOM
> + */
> +
> +dbus_bool_t
> +_dbus_get_standard_session_servicedirs (DBusList **dirs)
> +{
> + const char *xdg_data_home;
> + const char *xdg_data_dirs;
> + DBusString servicedir_path;
> +
> + if (!_dbus_string_init (&servicedir_path))
> + return FALSE;
> +
> + xdg_data_home = _dbus_getenv ("XDG_DATA_HOME");
> + xdg_data_dirs = _dbus_getenv ("XDG_DATA_DIRS");
> +
> + if (xdg_data_home != NULL)
> + {
> + if (!_dbus_string_append (&servicedir_path, xdg_data_home))
> + goto oom;
> +
> + if (!_dbus_string_append (&servicedir_path, ":"))
> + goto oom;
> + }
> + else
> + {
> + const DBusString *homedir;
> + const DBusString local_share;
> +
> + if (!_dbus_homedir_from_current_process (&homedir))
> + goto oom;
> +
> + if (!_dbus_string_append (&servicedir_path,
> _dbus_string_get_const_data (homedir)))
> + goto oom;
> +
> + _dbus_string_init_const (&local_share, "/.local/share:");
> + if (!_dbus_concat_dir_and_file (&servicedir_path,
> &local_share))
> + goto oom;
> + }
> +
> + /*
> + * add configured datadir to defaults
> + * this may be the same as an xdg dir
> + * however the config parser should take
> + * care of duplicates
> + */
> + if (!_dbus_string_append (&servicedir_path, DBUS_DATADIR":"))
> + goto oom;
> +
> + if (xdg_data_dirs != NULL)
> + {
> + if (!_dbus_string_append (&servicedir_path, xdg_data_dirs))
> + goto oom;
> + }
> + else
> + {
> + if (!_dbus_string_append (&servicedir_path, "/usr/share:"))
> + goto oom;
> +
> + if (!_dbus_string_append (&servicedir_path,
> "/usr/local/share"))
> + goto oom;
> + }
> +
> + if (!split_paths_and_append (&servicedir_path,
> +
> DBUS_UNIX_STANDARD_SESSION_SERVICEDIR,
> + dirs))
> + goto oom;
> +
> + _dbus_string_free (&servicedir_path);
> + return TRUE;
> +
> + oom:
> + _dbus_string_free (&servicedir_path);
> + return FALSE;
> +}
> +
> /* tests in dbus-sysdeps-util.c */
> Index: dbus/dbus-sysdeps.h
> ===================================================================
> RCS file: /cvs/dbus/dbus/dbus/dbus-sysdeps.h,v
> retrieving revision 1.65
> diff -u -p -r1.65 dbus-sysdeps.h
> --- dbus/dbus-sysdeps.h 27 Oct 2006 01:09:24 -0000 1.65
> +++ dbus/dbus-sysdeps.h 1 Nov 2006 20:32:55 -0000
> @@ -59,6 +59,9 @@ DBUS_BEGIN_DECLS
> /** An opaque string type */
> typedef struct DBusString DBusString;
>
> +/** avoid circular includes with DBusList */
> +typedef struct DBusList DBusList;
> +
> #if __GNUC__ > 2 || (__GNUC__ == 2 && __GNUC_MINOR__ > 4)
> #define _DBUS_GNUC_PRINTF( format_idx, arg_idx ) \
> __attribute__((__format__ (__printf__, format_idx, arg_idx)))
> @@ -291,6 +294,8 @@ dbus_bool_t _dbus_string_get_dirname (c
> DBusString *dirname);
> dbus_bool_t _dbus_path_is_absolute (const DBusString *filename);
>
> +dbus_bool_t _dbus_get_standard_session_servicedirs (DBusList
> **dirs);
> +
> /** Opaque type for reading a directory listing */
> typedef struct DBusDirIter DBusDirIter;
>
> Index: test/data/valid-config-files/many-rules.conf
> ===================================================================
> RCS file:
> /cvs/dbus/dbus/test/data/valid-config-files/many-rules.conf,v
> retrieving revision 1.4
> diff -u -p -r1.4 many-rules.conf
> --- test/data/valid-config-files/many-rules.conf 18 Jan 2005 20:42:15
> -0000 1.4
> +++ test/data/valid-config-files/many-rules.conf 1 Nov 2006 20:32:55
> -0000
> @@ -5,6 +5,7 @@
> <listen>unix:path=/foo/bar</listen>
> <listen>tcp:port=1234</listen>
> <includedir>basic.d</includedir>
> + <standard_session_servicedirs />
> <servicedir>/usr/share/foo</servicedir>
> <include ignore_missing="yes">nonexistent.conf</include>
> <policy context="default">
> > _______________________________________________
> dbus mailing list
> dbus at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dbus
>
___________________________________________________________
Der frühe Vogel fängt den Wurm. Hier gelangen Sie zum neuen Yahoo! Mail: http://mail.yahoo.de
More information about the dbus
mailing list