[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