2005-01-29 Havoc Pennington <hp@redhat.com>

	* glib/Makefile.am: rename dbus-glib-tool to dbus-binding-tool;
	though it uses glib, it could be extended for any binding in
	principle

	* glib/dbus-gobject.c (gobject_message_function): change to the
	new way properties work

	* dbus/dbus-protocol.h: add the new interfaces

	* doc/dbus-specification.xml: document the introspection format,
	Introspectable interface, and add an org.freedesktop.Properties
	interface.

	* glib/dbus-gparser.c: add support for a <property> element

	* glib/dbus-gidl.c: add PropertyInfo

	* glib/dbus-gobject.c (handle_introspect): put the outermost
	<node> outside the signal and property descriptions.
	(introspect_properties): export properties as <property> rather
	than as method calls

7 files changed, 434 insertions, 111 deletions

diff --git a/glib/.cvsignore b/glib/.cvsignore
index 881b4887..4e197a9f 100644
--- a/glib/.cvsignore
+++ b/glib/.cvsignore
@@ -5,7 +5,7 @@ Makefile.in
 *.lo
 *.la
 dbus-glib-test
-dbus-glib-tool
+dbus-binding-tool
 *.bb
 *.bbg
 *.da
diff --git a/glib/Makefile.am b/glib/Makefile.am
index e0362b42..4db47de4 100644
--- a/glib/Makefile.am
+++ b/glib/Makefile.am
@@ -34,12 +34,12 @@ libdbus_gtool_la_SOURCES =			\
 
 libdbus_gtool_la_LIBADD = libdbus-glib-1.la
 
-bin_PROGRAMS=dbus-glib-tool
+bin_PROGRAMS=dbus-binding-tool
 
-dbus_glib_tool_SOURCES =			\
+dbus_binding_tool_SOURCES =			\
 	dbus-glib-tool.c
 
-dbus_glib_tool_LDADD= -lexpat libdbus-gtool.la
+dbus_binding_tool_LDADD= -lexpat libdbus-gtool.la
 
 if DBUS_BUILD_TESTS
 
diff --git a/glib/dbus-gidl.c b/glib/dbus-gidl.c
index 6fa83916..cabc406b 100644
--- a/glib/dbus-gidl.c
+++ b/glib/dbus-gidl.c
@@ -2,7 +2,7 @@
 /* dbus-gidl.c data structure describing an interface, to be generated from IDL
  *             or something
  *
- * Copyright (C) 2003  Red Hat, Inc.
+ * Copyright (C) 2003, 2005  Red Hat, Inc.
  *
  * Licensed under the Academic Free License version 2.1
  *
@@ -46,6 +46,7 @@ struct InterfaceInfo
   /* Since we have BaseInfo now these could be one list */
   GSList *methods;
   GSList *signals;
+  GSList *properties;
 };
 
 struct MethodInfo
@@ -60,6 +61,13 @@ struct SignalInfo
   GSList *args;
 };
 
+struct PropertyInfo
+{
+  BaseInfo base;
+  int type;
+  PropertyAccessFlags access;
+};
+
 struct ArgInfo
 {
   BaseInfo base;
@@ -111,6 +119,9 @@ base_info_unref (BaseInfo *info)
     case INFO_TYPE_METHOD:
       method_info_unref ((MethodInfo*) info);
       break;
+    case INFO_TYPE_PROPERTY:
+      property_info_unref ((PropertyInfo*) info);
+      break;
     case INFO_TYPE_ARG:
       arg_info_unref ((ArgInfo*) info);
       break;
@@ -209,6 +220,20 @@ free_signal_list (GSList **signals_p)
   *signals_p = NULL;
 }
 
+static void
+free_property_list (GSList **props_p)
+{
+  GSList *tmp;
+  tmp = *props_p;
+  while (tmp != NULL)
+    {
+      property_info_unref (tmp->data);
+      tmp = tmp->next;
+    }
+  g_slist_free (*props_p);
+  *props_p = NULL;
+}
+
 NodeInfo*
 node_info_new (const char *name)
 {
@@ -307,6 +332,7 @@ interface_info_unref (InterfaceInfo *info)
     {
       free_method_list (&info->methods);
       free_signal_list (&info->signals);
+      free_property_list (&info->properties);
       base_info_free (info);
     }
 }
@@ -329,6 +355,12 @@ interface_info_get_signals (InterfaceInfo *info)
   return info->signals;
 }
 
+GSList*
+interface_info_get_properties (InterfaceInfo *info)
+{
+  return info->properties;
+}
+
 void
 interface_info_add_method (InterfaceInfo *info,
                            MethodInfo    *method)
@@ -345,6 +377,14 @@ interface_info_add_signal (InterfaceInfo *info,
   info->signals = g_slist_append (info->signals, signal);
 }
 
+void
+interface_info_add_property (InterfaceInfo *info,
+                             PropertyInfo  *property)
+{
+  property_info_ref (property);
+  info->properties = g_slist_append (info->properties, property);
+}
+
 static void
 free_arg_list (GSList **args_p)
 {
@@ -352,6 +392,8 @@ free_arg_list (GSList **args_p)
   tmp = *args_p;
   while (tmp != NULL)
     {
+      ArgInfo *ai = tmp->data;
+      g_assert (ai->base.type == INFO_TYPE_ARG);
       arg_info_unref (tmp->data);
       tmp = tmp->next;
     }
@@ -484,10 +526,64 @@ signal_info_add_arg (SignalInfo    *info,
   
   arg_info_ref (arg);
   info->args = g_slist_append (info->args, arg);
-
+  
   /* signal args don't need sorting since only "out" is allowed */
 }
 
+PropertyInfo*
+property_info_new (const char          *name,
+                   int                  type,
+                   PropertyAccessFlags  access)
+{
+  PropertyInfo *info;
+
+  info = g_new0 (PropertyInfo, 1);
+  info->base.refcount = 1;
+  info->base.name = g_strdup (name);
+  info->base.type = INFO_TYPE_PROPERTY;
+
+  info->type = type;
+  info->access = access;
+  
+  return info;
+}
+
+PropertyInfo*
+property_info_ref (PropertyInfo *info)
+{
+  info->base.refcount += 1;
+  
+  return info;
+}
+
+void
+property_info_unref (PropertyInfo *info)
+{
+  info->base.refcount -= 1;
+  if (info->base.refcount == 0)
+    {
+      base_info_free (info);
+    }
+}
+
+const char*
+property_info_get_name (PropertyInfo *info)
+{
+  return info->base.name;
+}
+
+int
+property_info_get_type (PropertyInfo *info)
+{
+  return info->type;
+}
+
+PropertyAccessFlags
+property_info_get_access (PropertyInfo *info)
+{
+  return info->access;
+}
+
 ArgInfo*
 arg_info_new (const char  *name,
               ArgDirection direction,
diff --git a/glib/dbus-gidl.h b/glib/dbus-gidl.h
index 47b4f093..fffbddfc 100644
--- a/glib/dbus-gidl.h
+++ b/glib/dbus-gidl.h
@@ -36,21 +36,30 @@ typedef struct NodeInfo      NodeInfo;
 typedef struct InterfaceInfo InterfaceInfo;
 typedef struct MethodInfo    MethodInfo;
 typedef struct SignalInfo    SignalInfo;
+typedef struct PropertyInfo  PropertyInfo;
 typedef struct ArgInfo       ArgInfo;
 
 typedef enum
 {
+  ARG_INVALID = -1,
   ARG_IN,
   ARG_OUT
 } ArgDirection;
 
 typedef enum
 {
+  PROPERTY_READ  = 1 << 0,
+  PROPERTY_WRITE = 1 << 1
+} PropertyAccessFlags;
+
+typedef enum
+{
   INFO_TYPE_NODE,
   INFO_TYPE_INTERFACE,
   INFO_TYPE_METHOD,
   INFO_TYPE_SIGNAL,
-  INFO_TYPE_ARG
+  INFO_TYPE_ARG,
+  INFO_TYPE_PROPERTY
 
 } InfoType;
 
@@ -64,54 +73,59 @@ GType          base_info_get_gtype        (void);
 #define        BASE_INFO_TYPE             (base_info_get_gtype ())
 
 
-NodeInfo*      node_info_new              (const char    *name);
-NodeInfo*      node_info_ref              (NodeInfo      *info);
-void           node_info_unref            (NodeInfo      *info);
-const char*    node_info_get_name         (NodeInfo      *info);
-GSList*        node_info_get_interfaces   (NodeInfo      *info);
-GSList*        node_info_get_nodes        (NodeInfo      *info);
-void           node_info_add_interface    (NodeInfo      *info,
-                                           InterfaceInfo *interface);
-void           node_info_add_node         (NodeInfo      *info,
-                                           NodeInfo      *child);
-
-InterfaceInfo* interface_info_new         (const char    *name);
-InterfaceInfo* interface_info_ref         (InterfaceInfo *info);
-void           interface_info_unref       (InterfaceInfo *info);
-const char*    interface_info_get_name    (InterfaceInfo *info);
-GSList*        interface_info_get_methods (InterfaceInfo *info);
-GSList*        interface_info_get_signals (InterfaceInfo *info);
-void           interface_info_add_method  (InterfaceInfo *info,
-                                           MethodInfo    *method);
-void           interface_info_add_signal  (InterfaceInfo *info,
-                                           SignalInfo    *signal);
-
-MethodInfo*    method_info_new            (const char    *name);
-MethodInfo*    method_info_ref            (MethodInfo    *info);
-void           method_info_unref          (MethodInfo    *info);
-
-const char*    method_info_get_name       (MethodInfo    *info);
-GSList*        method_info_get_args       (MethodInfo    *info);
-void           method_info_add_arg        (MethodInfo    *info,
-                                           ArgInfo       *arg);
-
-SignalInfo*    signal_info_new            (const char    *name);
-SignalInfo*    signal_info_ref            (SignalInfo    *info);
-void           signal_info_unref          (SignalInfo    *info);
-
-const char*    signal_info_get_name       (SignalInfo    *info);
-GSList*        signal_info_get_args       (SignalInfo    *info);
-void           signal_info_add_arg        (SignalInfo    *info,
-                                           ArgInfo       *arg);
-
-ArgInfo*       arg_info_new               (const char    *name,
-                                           ArgDirection   direction,
-                                           int            type);
-ArgInfo*       arg_info_ref               (ArgInfo       *info);
-void           arg_info_unref             (ArgInfo       *info);
-const char*    arg_info_get_name          (ArgInfo       *info);
-int            arg_info_get_type          (ArgInfo       *info);
-ArgDirection   arg_info_get_direction     (ArgInfo       *info);
+NodeInfo*           node_info_new                 (const char          *name);
+NodeInfo*           node_info_ref                 (NodeInfo            *info);
+void                node_info_unref               (NodeInfo            *info);
+const char*         node_info_get_name            (NodeInfo            *info);
+GSList*             node_info_get_interfaces      (NodeInfo            *info);
+GSList*             node_info_get_nodes           (NodeInfo            *info);
+void                node_info_add_interface       (NodeInfo            *info,
+                                                   InterfaceInfo       *interface);
+void                node_info_add_node            (NodeInfo            *info,
+                                                   NodeInfo            *child);
+InterfaceInfo*      interface_info_new            (const char          *name);
+InterfaceInfo*      interface_info_ref            (InterfaceInfo       *info);
+void                interface_info_unref          (InterfaceInfo       *info);
+const char*         interface_info_get_name       (InterfaceInfo       *info);
+GSList*             interface_info_get_methods    (InterfaceInfo       *info);
+GSList*             interface_info_get_signals    (InterfaceInfo       *info);
+GSList*             interface_info_get_properties (InterfaceInfo       *info);
+void                interface_info_add_method     (InterfaceInfo       *info,
+                                                   MethodInfo          *method);
+void                interface_info_add_signal     (InterfaceInfo       *info,
+                                                   SignalInfo          *signal);
+void                interface_info_add_property   (InterfaceInfo       *info,
+                                                   PropertyInfo        *property);
+MethodInfo*         method_info_new               (const char          *name);
+MethodInfo*         method_info_ref               (MethodInfo          *info);
+void                method_info_unref             (MethodInfo          *info);
+const char*         method_info_get_name          (MethodInfo          *info);
+GSList*             method_info_get_args          (MethodInfo          *info);
+void                method_info_add_arg           (MethodInfo          *info,
+                                                   ArgInfo             *arg);
+SignalInfo*         signal_info_new               (const char          *name);
+SignalInfo*         signal_info_ref               (SignalInfo          *info);
+void                signal_info_unref             (SignalInfo          *info);
+const char*         signal_info_get_name          (SignalInfo          *info);
+GSList*             signal_info_get_args          (SignalInfo          *info);
+void                signal_info_add_arg           (SignalInfo          *info,
+                                                   ArgInfo             *arg);
+PropertyInfo*       property_info_new             (const char          *name,
+                                                   int                  type,
+                                                   PropertyAccessFlags  access);
+PropertyInfo*       property_info_ref             (PropertyInfo        *info);
+void                property_info_unref           (PropertyInfo        *info);
+const char*         property_info_get_name        (PropertyInfo        *info);
+int                 property_info_get_type        (PropertyInfo        *info);
+PropertyAccessFlags property_info_get_access      (PropertyInfo        *info);
+ArgInfo*            arg_info_new                  (const char          *name,
+                                                   ArgDirection         direction,
+                                                   int                  type);
+ArgInfo*            arg_info_ref                  (ArgInfo             *info);
+void                arg_info_unref                (ArgInfo             *info);
+const char*         arg_info_get_name             (ArgInfo             *info);
+int                 arg_info_get_type             (ArgInfo             *info);
+ArgDirection        arg_info_get_direction        (ArgInfo             *info);
 
 G_END_DECLS
 
diff --git a/glib/dbus-glib-tool.c b/glib/dbus-glib-tool.c
index c460a352..c8273afa 100644
--- a/glib/dbus-glib-tool.c
+++ b/glib/dbus-glib-tool.c
@@ -57,7 +57,7 @@ pretty_print_list (GSList *list,
                    int     depth)
 {
   GSList *tmp;
-
+  
   tmp = list;
   while (tmp != NULL)
     {
@@ -106,6 +106,7 @@ pretty_print (BaseInfo *base,
 
         pretty_print_list (interface_info_get_methods (i), depth + 1);
         pretty_print_list (interface_info_get_signals (i), depth + 1);
+        pretty_print_list (interface_info_get_properties (i), depth + 1);
 
         indent (depth);
         printf ("}\n");
@@ -139,6 +140,22 @@ pretty_print (BaseInfo *base,
         printf (")\n");
       }
       break;
+    case INFO_TYPE_PROPERTY:
+      {
+        PropertyInfo *a = (PropertyInfo*) base;
+        int pt = property_info_get_type (a);
+        PropertyAccessFlags acc = property_info_get_access (a);
+
+        printf ("%s%s %s",
+                acc & PROPERTY_READ ? "read" : "",
+                acc & PROPERTY_WRITE ? "write" : "",
+                _dbus_gutils_type_to_string (pt));
+        if (name)
+          printf (" %s\n", name);
+        else
+          printf ("\n");
+      }
+      break;
     case INFO_TYPE_ARG:
       {
         ArgInfo *a = (ArgInfo*) base;
@@ -160,16 +177,16 @@ pretty_print (BaseInfo *base,
 static void
 usage (int ecode)
 {
-  fprintf (stderr, "dbus-glib-tool [--version] [--help]\n");
+  fprintf (stderr, "dbus-binding-tool [--version] [--help] [--pretty-print]\n");
   exit (ecode);
 }
 
 static void
 version (void)
 {
-  printf ("D-BUS GLib Tool %s\n"
-          "Copyright (C) 2003, 2004 Red Hat, Inc.\n"
-          "This is free software; see the source for copying conditions.\n"
+  printf ("D-BUS Binding Tool %s\n"
+          "Copyright (C) 2003-2005 Red Hat, Inc.\n"
+          "This is free software; xsee the source for copying conditions.\n"
           "There is NO warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.\n",
           VERSION);
   exit (0);
@@ -313,11 +330,11 @@ run_all_tests (const char *test_data_dir)
   else
     printf ("No test data!\n");
 
-  printf ("%s: running gtool tests\n", "dbus-glib-tool");
+  printf ("%s: running binding tests\n", "dbus-binding-tool");
   if (!_dbus_gtool_test (test_data_dir))
     test_die ("gtool");
 
-  printf ("%s: completed successfully\n", "dbus-glib-tool");
+  printf ("%s: completed successfully\n", "dbus-binding-tool");
 }
 
 #endif /* DBUS_BUILD_TESTS */
diff --git a/glib/dbus-gobject.c b/glib/dbus-gobject.c
index 18361035..5aab4a68 100644
--- a/glib/dbus-gobject.c
+++ b/glib/dbus-gobject.c
@@ -198,28 +198,27 @@ introspect_properties (GObject *object, GString *xml)
 
       s = uscore_to_wincaps (spec->name);
       
-      if (can_set)
+      if (can_set || can_get)
         {
-          g_string_append (xml, "    <method name=\"set_");
+          g_string_append (xml, "    <property name=\"");
           g_string_append (xml, s);
-          g_string_append (xml, "\">\n");
-          
-          g_string_append (xml, "      <arg type=\"");
+          g_string_append (xml, "\" type=\"");
           g_string_append (xml, _dbus_gutils_type_to_string (dbus_type));
-          g_string_append (xml, "\"/>\n");
-        }
+          g_string_append (xml, "\" access=\"\n");
 
-      if (can_get)
-        {
-          g_string_append (xml, "    <method name=\"get_");
-          g_string_append (xml, s);
-          g_string_append (xml, "\">\n");
+          if (can_set && can_get)
+            g_string_append (xml, "readwrite");
+          else if (can_get)
+            g_string_append (xml, "read");
+          else
+            {
+              g_assert (can_set);
+              g_string_append (xml, "write");
+            }
           
-          g_string_append (xml, "      <arg type=\"");
-          g_string_append (xml, _dbus_gutils_type_to_string (dbus_type));
-          g_string_append (xml, "\" direction=\"out\"/>\n");
+          g_string_append (xml, "\">\n");
         }
-
+      
       g_free (s);
     }
 
@@ -290,10 +289,10 @@ handle_introspect (DBusConnection *connection,
   
   xml = g_string_new (NULL);
 
+  g_string_append (xml, "<node>\n");
+  
   introspect_signals (G_OBJECT_TYPE (object), xml);
   introspect_properties (object, xml);
-	  
-  g_string_append (xml, "<node>\n");
 
   /* Append child nodes */
   for (i = 0; children[i]; i++)
@@ -322,23 +321,24 @@ handle_introspect (DBusConnection *connection,
 }
 
 static DBusMessage*
-set_object_property (DBusConnection *connection,
-                     DBusMessage    *message,
-                     GObject        *object,
-                     GParamSpec     *pspec)
+set_object_property (DBusConnection  *connection,
+                     DBusMessage     *message,
+                     DBusMessageIter *iter,
+                     GObject         *object,
+                     GParamSpec      *pspec)
 {
   GValue value = { 0, };
   DBusMessage *ret;
-  DBusMessageIter iter;
-
-  dbus_message_iter_init (message, &iter);
+  DBusMessageIter sub;
 
+  dbus_message_iter_recurse (iter, &sub);
+  
   /* The g_object_set_property() will transform some types, e.g. it
    * will let you use a uchar to set an int property etc. Note that
    * any error in value range or value conversion will just
    * g_warning(). These GObject skels are not for secure applications.
    */
-  if (dbus_gvalue_demarshal (&iter, &value))
+  if (dbus_gvalue_demarshal (&sub, &value))
     {
       g_object_set_property (object,
                              pspec->name,
@@ -405,10 +405,12 @@ gobject_message_function (DBusConnection  *connection,
   const DBusGObjectInfo *info;
   GParamSpec *pspec;
   GObject *object;
-  const char *member;
   gboolean setter;
   gboolean getter;
   char *s;
+  const char *wincaps_propname;
+  const char *wincaps_propiface;
+  DBusMessageIter iter;
 
   object = G_OBJECT (user_data);
 
@@ -417,8 +419,6 @@ gobject_message_function (DBusConnection  *connection,
                                    "Introspect"))
     return handle_introspect (connection, message, object);
 
-  member = dbus_message_get_member (message);
-
   /* Try the metainfo, which lets us invoke methods */
 
   g_static_mutex_lock (&info_hash_mutex);
@@ -439,13 +439,39 @@ gobject_message_function (DBusConnection  *connection,
   /* If no metainfo, we can still do properties and signals
    * via standard GLib introspection
    */
-  setter = (member[0] == 's' && member[1] == 'e' && member[2] == 't' && member[3] == '_');
-  getter = (member[0] == 'g' && member[1] == 'e' && member[2] == 't' && member[3] == '_');
+  getter = FALSE;
+  setter = FALSE;
+  if (dbus_message_is_method_call (message,
+                                   DBUS_INTERFACE_ORG_FREEDESKTOP_PROPERTIES,
+                                   "Get"))
+    getter = TRUE;
+  else if (dbus_message_is_method_call (message,
+                                        DBUS_INTERFACE_ORG_FREEDESKTOP_PROPERTIES,
+                                        "Set"))
+    setter = TRUE;
 
   if (!(setter || getter))
     return DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
 
-  s = wincaps_to_uscore (&member[4]);
+  dbus_message_iter_init (message, &iter);
+
+  if (dbus_message_iter_get_arg_type (&iter) != DBUS_TYPE_STRING)
+    {
+      g_warning ("Property get or set does not have an interface string as first arg\n");
+      return DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
+    }
+  dbus_message_iter_get_basic (&iter, &wincaps_propiface);
+  dbus_message_iter_next (&iter);
+
+  if (dbus_message_iter_get_arg_type (&iter) != DBUS_TYPE_STRING)
+    {
+      g_warning ("Property get or set does not have a property name string as second arg\n");
+      return DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
+    }
+  dbus_message_iter_get_basic (&iter, &wincaps_propname);
+  dbus_message_iter_next (&iter);
+  
+  s = wincaps_to_uscore (wincaps_propname);
 
   pspec = g_object_class_find_property (G_OBJECT_GET_CLASS (object),
                                         s);
@@ -458,11 +484,18 @@ gobject_message_function (DBusConnection  *connection,
 
       if (setter)
         {
-          ret = set_object_property (connection, message,
+          if (dbus_message_iter_get_arg_type (&iter) != DBUS_TYPE_VARIANT)
+            {
+              g_warning ("Property set does not have a variant value as third arg\n");
+              return DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
+            }
+          
+          ret = set_object_property (connection, message, &iter,
                                      object, pspec);
+          dbus_message_iter_next (&iter);
         }
       else if (getter)
-        {
+        {     
           ret = get_object_property (connection, message,
                                      object, pspec);
         }
@@ -474,6 +507,9 @@ gobject_message_function (DBusConnection  *connection,
 
       g_assert (ret != NULL);
 
+      if (dbus_message_iter_get_arg_type (&iter) != DBUS_TYPE_INVALID)
+        g_warning ("Property get or set had too many arguments\n");
+      
       dbus_connection_send (connection, ret, NULL);
       dbus_message_unref (ret);
       return DBUS_HANDLER_RESULT_HANDLED;
diff --git a/glib/dbus-gparser.c b/glib/dbus-gparser.c
index 478dcdf3..83922af4 100644
--- a/glib/dbus-gparser.c
+++ b/glib/dbus-gparser.c
@@ -1,7 +1,7 @@
 /* -*- mode: C; c-file-style: "gnu" -*- */
 /* dbus-gparser.c parse DBus description files
  *
- * Copyright (C) 2003  Red Hat, Inc.
+ * Copyright (C) 2003, 2005  Red Hat, Inc.
  *
  * Licensed under the Academic Free License version 2.1
  * 
@@ -169,6 +169,7 @@ struct Parser
   InterfaceInfo *interface;
   MethodInfo *method;
   SignalInfo *signal;
+  PropertyInfo *property;
   ArgInfo *arg;
 };
 
@@ -238,11 +239,12 @@ parse_node (Parser      *parser,
   if (parser->interface ||
       parser->method ||
       parser->signal ||
+      parser->property ||
       parser->arg)
     {
       g_set_error (error, G_MARKUP_ERROR,
                    G_MARKUP_ERROR_PARSE,
-                   _("Can't put a <%s> element here"),
+                   _("Can't put <%s> element here"),
                    element_name);
       return FALSE;      
     }
@@ -264,6 +266,25 @@ parse_node (Parser      *parser,
       return FALSE;
     }
 
+  /* Root element name must be absolute */
+  if (parser->node_stack == NULL && name && *name != '/')
+    {
+      g_set_error (error, G_MARKUP_ERROR,
+                   G_MARKUP_ERROR_PARSE,
+                   _("\"%s\" attribute on <%s> element must be an absolute object path, \"%s\" not OK"),
+                   "name", element_name, name);
+      return FALSE;
+    }
+
+  /* Other element names must not be absolute */
+  if (parser->node_stack != NULL && name && *name == '/')
+    {
+      g_set_error (error, G_MARKUP_ERROR,
+                   G_MARKUP_ERROR_PARSE,
+                   _("\"%s\" attribute on <%s> element must not be an absolute object path, \"%s\" starts with /"),
+                   "name", element_name, name);
+      return FALSE;
+    }
   
   node = node_info_new (name);
 
@@ -293,12 +314,13 @@ parse_interface (Parser      *parser,
   if (parser->interface ||
       parser->method ||
       parser->signal ||
+      parser->property ||
       parser->arg ||
       (parser->node_stack == NULL))
     {
       g_set_error (error, G_MARKUP_ERROR,
                    G_MARKUP_ERROR_PARSE,
-                   _("Can't put a <%s> element here"),
+                   _("Can't put <%s> element here"),
                    element_name);
       return FALSE;      
     }
@@ -345,11 +367,12 @@ parse_method (Parser      *parser,
       parser->node_stack == NULL ||
       parser->method ||
       parser->signal ||
+      parser->property ||
       parser->arg)
     {
       g_set_error (error, G_MARKUP_ERROR,
                    G_MARKUP_ERROR_PARSE,
-                   _("Can't put a <%s> element here"),
+                   _("Can't put <%s> element here"),
                    element_name);
       return FALSE;      
     }
@@ -395,12 +418,13 @@ parse_signal (Parser      *parser,
   if (parser->interface == NULL ||
       parser->node_stack == NULL ||
       parser->signal ||
-      parser->signal ||
+      parser->method ||
+      parser->property ||
       parser->arg)
     {
       g_set_error (error, G_MARKUP_ERROR,
                    G_MARKUP_ERROR_PARSE,
-                   _("Can't put a <%s> element here"),
+                   _("Can't put <%s> element here"),
                    element_name);
       return FALSE;      
     }
@@ -463,10 +487,122 @@ basic_type_from_string (const char *str)
     return DBUS_TYPE_INVALID;
 }
 
+/* FIXME we have to allow type signatures, not just basic types
+ */
 static int
-type_from_string (const char *str)
+type_from_string (const char *str,
+                  const char *element_name,
+                  GError    **error)
+{
+  int t;
+  
+  t = basic_type_from_string (str);
+
+  if (t == DBUS_TYPE_INVALID)
+    {
+      g_set_error (error, G_MARKUP_ERROR,
+                   G_MARKUP_ERROR_PARSE,
+                   _("Type \"%s\" not understood on <%s> element "),
+                   str, element_name);
+    }
+
+  return t;
+}
+
+static gboolean
+parse_property (Parser      *parser,
+                const char  *element_name,
+                const char **attribute_names,
+                const char **attribute_values,
+                GError     **error)
 {
-  return basic_type_from_string (str);
+  const char *name;
+  const char *access;
+  const char *type;
+  PropertyInfo *property;
+  NodeInfo *top;
+  PropertyAccessFlags access_flags;
+  int t;
+  
+  if (parser->interface == NULL ||
+      parser->node_stack == NULL ||
+      parser->signal ||
+      parser->method ||
+      parser->property ||
+      parser->arg)
+    {
+      g_set_error (error, G_MARKUP_ERROR,
+                   G_MARKUP_ERROR_PARSE,
+                   _("Can't put <%s> element here"),
+                   element_name);
+      return FALSE;      
+    }
+
+  name = NULL;
+  if (!locate_attributes (element_name, attribute_names,
+                          attribute_values, error,
+                          "name", &name,
+                          "access", &access,
+                          "type", &type,
+                          NULL))
+    return FALSE;
+
+  if (name == NULL)
+    {
+      g_set_error (error, G_MARKUP_ERROR,
+                   G_MARKUP_ERROR_PARSE,
+                   _("\"%s\" attribute required on <%s> element "),
+                   "name", element_name);
+      return FALSE;
+    }
+
+  if (access == NULL)
+    {
+      g_set_error (error, G_MARKUP_ERROR,
+                   G_MARKUP_ERROR_PARSE,
+                   _("\"%s\" attribute required on <%s> element "),
+                   "access", element_name);
+      return FALSE;
+    }
+
+  if (type == NULL)
+    {
+      g_set_error (error, G_MARKUP_ERROR,
+                   G_MARKUP_ERROR_PARSE,
+                   _("\"%s\" attribute required on <%s> element "),
+                   "type", element_name);
+      return FALSE;
+    }
+
+  t = type_from_string (type, element_name, error);
+  if (t == DBUS_TYPE_INVALID)
+    return FALSE;
+
+  access_flags = 0;
+  if (strcmp (access, "readwrite") == 0)
+    access_flags = PROPERTY_READ | PROPERTY_WRITE;
+  else if (strcmp (access, "read") == 0)
+    access_flags = PROPERTY_READ;
+  else if (strcmp (access, "write") == 0)
+    access_flags = PROPERTY_WRITE;
+  else
+    {
+      g_set_error (error, G_MARKUP_ERROR,
+                   G_MARKUP_ERROR_PARSE,
+                   _("access=\"%s\" must have value readwrite, read, or write on %s\n"),
+                   access, element_name);
+      return FALSE;
+    }
+  
+  top = parser->node_stack->data;
+  
+  property = property_info_new (name, t, access_flags);
+  interface_info_add_property (parser->interface, property);
+  property_info_unref (property);
+
+  parser->property = property;
+  
+  return TRUE;
 }
 
 static gboolean
@@ -485,11 +621,12 @@ parse_arg (Parser      *parser,
   
   if (!(parser->method || parser->signal) ||
       parser->node_stack == NULL ||
+      parser->property ||
       parser->arg)
     {
       g_set_error (error, G_MARKUP_ERROR,
                    G_MARKUP_ERROR_PARSE,
-                   _("Can't put a <%s> element here"),
+                   _("Can't put <%s> element here"),
                    element_name);
       return FALSE;      
     }
@@ -525,20 +662,31 @@ parse_arg (Parser      *parser,
         g_assert_not_reached ();
     }
 
+  dir = ARG_INVALID;
+  
   if (strcmp (direction, "in") == 0)
     dir = ARG_IN;
   else if (strcmp (direction, "out") == 0)
     dir = ARG_OUT;
-  else
+  
+  if (dir == ARG_INVALID ||
+      (parser->signal && dir == ARG_IN))
     {
-      g_set_error (error, G_MARKUP_ERROR,
-                   G_MARKUP_ERROR_PARSE,
-                   _("\"%s\" attribute on <%s> has value \"in\" or \"out\""),
-                   "direction", element_name);
+      if (parser->signal)
+        g_set_error (error, G_MARKUP_ERROR,
+                     G_MARKUP_ERROR_PARSE,
+                     _("Signals must have direction=\"out\" (just omit the direction attribute)"));
+      else
+        g_set_error (error, G_MARKUP_ERROR,
+                     G_MARKUP_ERROR_PARSE,
+                     _("\"%s\" attribute on <%s> has value \"in\" or \"out\""),
+                     "direction", element_name);
       return FALSE;
     }
 
-  t = type_from_string (type);
+  t = type_from_string (type, element_name, error);
+  if (t == DBUS_TYPE_INVALID)
+    return FALSE;
   
   arg = arg_info_new (name, dir, t);
   if (parser->method)
@@ -588,6 +736,12 @@ parser_start_element (Parser      *parser,
                          attribute_values, error))
         return FALSE;
     }
+  else if (ELEMENT_IS ("property"))
+    {
+      if (!parse_property (parser, element_name, attribute_names,
+                           attribute_values, error))
+        return FALSE;
+    }
   else if (ELEMENT_IS ("arg"))
     {
       if (!parse_arg (parser, element_name, attribute_names,
@@ -624,6 +778,10 @@ parser_end_element (Parser      *parser,
     {
       parser->signal = NULL;
     }
+  else if (ELEMENT_IS ("property"))
+    {
+      parser->property = NULL;
+    }
   else if (ELEMENT_IS ("arg"))
     {
       parser->arg = NULL;
@@ -655,6 +813,8 @@ parser_content (Parser      *parser,
 {
   g_return_val_if_fail (error == NULL || *error == NULL, FALSE);
 
+  /* FIXME check that it's all whitespace */
+  
   return TRUE;
 }