Use D-Bus properties instead of GetProperties

- Use D-Bus native properties instead of a GetProperties call.
- Fix a number of front-ends by registering the right signals and
marshallers following the "done" signal argument addition
- Fix VerifyStart call in the pam module

TODO

@@ -7,9 +7,6 @@ Enforce command sequences:
 
 Verify PAM messages fit with GDM/gnome-screensaver
 
-Add API docs (see doc/dbus and doc subdir):
-http://gitweb.freedesktop.org/?p=DeviceKit/DeviceKit-disks.git;a=tree
-
 Register fprintd' po file with Transifex, Rosetta or the Translation Project
 
 Support insertion/removal of devices
@@ -24,4 +21,3 @@ Add POS use case
 
 Review error cases, and possible errors
 
-Use property instead of a GetProperties method for devices

pam/Makefile.am

@@ -3,11 +3,20 @@ if HAVE_PAM
 pammod_PROGRAMS = pam_fprintd.so
 pammoddir=$(libdir)/security
 
-pam_fprintd_so_SOURCES = pam_fprintd.c
+pam_fprintd_so_SOURCES = pam_fprintd.c $(MARSHALFILES)
 pam_fprintd_so_CFLAGS = -fPIC $(WARN_CFLAGS) $(GLIB_CFLAGS)
 pam_fprintd_so_LDFLAGS = -shared
 pam_fprintd_so_LDADD = $(PAM_LIBS) $(GLIB_LIBS)
 
+MARSHALFILES = marshal.c marshal.h
+GLIB_GENMARSHAL=`pkg-config --variable=glib_genmarshal glib-2.0`
+BUILT_SOURCES = $(MARSHALFILES)
+
+marshal.h: $(top_srcdir)/src/fprintd-marshal.list
+	( $(GLIB_GENMARSHAL) --prefix=fprintd_marshal $(top_srcdir)/src/fprintd-marshal.list --header > marshal.h )
+marshal.c: marshal.h
+	( $(GLIB_GENMARSHAL) --prefix=fprintd_marshal $(top_srcdir)/src/fprintd-marshal.list --body --header > marshal.c )
+
 endif
 
 EXTRA_DIST = pam_fprintd.c

pam/pam_fprintd.c

@@ -32,6 +32,8 @@
 #define PAM_SM_AUTH
 #include <security/pam_modules.h>
 
+#include "marshal.h"
+
 #define MAX_TRIES 3
 #define TIMEOUT 30
 
@@ -303,6 +305,7 @@ static int do_verify(DBusGConnection *connection, GMainLoop *loop, pam_handle_t
 {
 	GError *error;
 	GHashTable *props;
+	DBusGProxy *p;
 	verify_data *data;
 	int ret;
 
@@ -311,20 +314,28 @@ static int do_verify(DBusGConnection *connection, GMainLoop *loop, pam_handle_t
 	data->pamh = pamh;
 	data->loop = loop;
 
-	if (dbus_g_proxy_call (dev, "GetProperties", &error, G_TYPE_INVALID,
+	/* Get some properties for the device */
-			       dbus_g_type_get_map ("GHashTable", G_TYPE_STRING, G_TYPE_STRING), &props, G_TYPE_INVALID)) {
+	p = dbus_g_proxy_new_for_name(connection,
+				      "net.reactivated.Fprint", dbus_g_proxy_get_path (dev),
+				      "org.freedesktop.DBus.Properties");
+
+	if (dbus_g_proxy_call (p, "GetAll", &error, G_TYPE_INVALID,
+			       dbus_g_type_get_map ("GHashTable", G_TYPE_STRING, G_TYPE_VALUE), &props, G_TYPE_INVALID)) {
 		const char *scan_type;
-		data->driver = g_value_dup_string (g_hash_table_lookup (props, "Name"));
+		data->driver = g_value_dup_string (g_hash_table_lookup (props, "name"));
-		scan_type = g_value_dup_string (g_hash_table_lookup (props, "ScanType"));
+		scan_type = g_value_dup_string (g_hash_table_lookup (props, "scan-type"));
 		if (g_str_equal (scan_type, "swipe"))
 			data->is_swipe = TRUE;
 		g_hash_table_destroy (props);
 	}
+
+	g_object_unref (p);
+
 	if (!data->driver)
 		data->driver = g_strdup ("Fingerprint reader");
 
-	dbus_g_proxy_add_signal(dev, "VerifyStatus", G_TYPE_INT, NULL);
+	dbus_g_proxy_add_signal(dev, "VerifyStatus", G_TYPE_STRING, G_TYPE_BOOLEAN, NULL);
-	dbus_g_proxy_add_signal(dev, "VerifyFingerSelected", G_TYPE_INT, NULL);
+	dbus_g_proxy_add_signal(dev, "VerifyFingerSelected", G_TYPE_STRING, NULL);
 	dbus_g_proxy_connect_signal(dev, "VerifyStatus", G_CALLBACK(verify_result),
 				    data, NULL);
 	dbus_g_proxy_connect_signal(dev, "VerifyFingerSelected", G_CALLBACK(verify_finger_selected),
@@ -343,7 +354,7 @@ static int do_verify(DBusGConnection *connection, GMainLoop *loop, pam_handle_t
 
 		data->timed_out = FALSE;
 
-		if (!dbus_g_proxy_call (dev, "VerifyStart", &error, G_TYPE_UINT, -1, G_TYPE_INVALID, G_TYPE_INVALID)) {
+		if (!dbus_g_proxy_call (dev, "VerifyStart", &error, G_TYPE_STRING, "any", G_TYPE_INVALID, G_TYPE_INVALID)) {
 			D(g_message("VerifyStart failed: %s", error->message));
 			g_error_free (error);
 
@@ -434,6 +445,9 @@ PAM_EXTERN int pam_sm_authenticate(pam_handle_t *pamh, int flags, int argc,
 
 	g_type_init ();
 
+	dbus_g_object_register_marshaller (fprintd_marshal_VOID__STRING_BOOLEAN,
+					   G_TYPE_NONE, G_TYPE_STRING, G_TYPE_BOOLEAN, G_TYPE_INVALID);
+
 	pam_get_item(pamh, PAM_RHOST, (const void **)(const void*) &rhost);
 	if (rhost != NULL && strlen(rhost) > 0) {
 		/* remote login (e.g. over SSH) */

src/device.c

@@ -68,8 +68,6 @@ static void fprint_device_list_enrolled_fingers(FprintDevice *rdev,
 static void fprint_device_delete_enrolled_fingers(FprintDevice *rdev,
 						  const char *username,
 						  DBusGMethodInvocation *context);
-static void fprint_device_get_properties (FprintDevice *rdev,
-					  DBusGMethodInvocation *context);
 
 #include "device-dbus-glue.h"
 
@@ -123,6 +121,9 @@ typedef struct FprintDevicePrivate FprintDevicePrivate;
 enum fprint_device_properties {
 	FPRINT_DEVICE_CONSTRUCT_DDEV = 1,
 	FPRINT_DEVICE_IN_USE,
+	FPRINT_DEVICE_NAME,
+	FPRINT_DEVICE_NUM_ENROLL,
+	FPRINT_DEVICE_SCAN_TYPE
 };
 
 enum fprint_device_signals {
@@ -171,6 +172,26 @@ static void fprint_device_get_property(GObject *object, guint property_id,
 	case FPRINT_DEVICE_IN_USE:
 		g_value_set_boolean(value, g_hash_table_size (priv->clients) != 0);
 		break;
+	case FPRINT_DEVICE_NAME:
+		g_value_set_static_string (value, fp_driver_get_full_name (fp_dscv_dev_get_driver (priv->ddev)));
+		break;
+	case FPRINT_DEVICE_NUM_ENROLL:
+		if (priv->dev)
+			g_value_set_int (value, fp_dev_get_nr_enroll_stages (priv->dev));
+		else
+			g_value_set_int (value, -1);
+		break;
+	case FPRINT_DEVICE_SCAN_TYPE: {
+		const char *type;
+
+		if (fp_driver_get_scan_type (fp_dscv_dev_get_driver (priv->ddev)) == FP_SCAN_TYPE_PRESS)
+			type = "press";
+		else
+			type = "swipe";
+
+		g_value_set_static_string (value, type);
+		break;
+	}
 	default:
 		G_OBJECT_WARN_INVALID_PROPERTY_ID(object, property_id, pspec);
 		break;
@@ -196,12 +217,31 @@ static void fprint_device_class_init(FprintDeviceClass *klass)
 				     G_PARAM_CONSTRUCT_ONLY | G_PARAM_WRITABLE);
 	g_object_class_install_property(gobject_class,
 					FPRINT_DEVICE_CONSTRUCT_DDEV, pspec);
+
 	pspec = g_param_spec_boolean("in-use", "In use",
 				     "Whether the device is currently in use", FALSE,
 				     G_PARAM_READABLE);
 	g_object_class_install_property(gobject_class,
 					FPRINT_DEVICE_IN_USE, pspec);
 
+	pspec = g_param_spec_string("name", "Name",
+				    "The product name of the device", NULL,
+				    G_PARAM_READABLE);
+	g_object_class_install_property(gobject_class,
+					FPRINT_DEVICE_NAME, pspec);
+
+	pspec = g_param_spec_string("scan-type", "Scan Type",
+				    "The scan type of the device", "press",
+				    G_PARAM_READABLE);
+	g_object_class_install_property(gobject_class,
+					FPRINT_DEVICE_SCAN_TYPE, pspec);
+
+	pspec = g_param_spec_int("num-enroll-stages", "Number of enrollments stages",
+				  "Number of enrollment stages for the device.",
+				  -1, G_MAXINT, -1, G_PARAM_READABLE);
+	g_object_class_install_property(gobject_class,
+					FPRINT_DEVICE_NUM_ENROLL, pspec);
+
 	signals[SIGNAL_VERIFY_STATUS] = g_signal_new("verify-status",
 		G_TYPE_FROM_CLASS(gobject_class), G_SIGNAL_RUN_LAST, 0, NULL, NULL,
 		fprintd_marshal_VOID__STRING_BOOLEAN, G_TYPE_NONE, 2, G_TYPE_STRING, G_TYPE_BOOLEAN);
@@ -1147,35 +1187,3 @@ static void fprint_device_delete_enrolled_fingers(FprintDevice *rdev,
 	dbus_g_method_return(context);
 }
 
-static void fprint_device_get_properties (FprintDevice *rdev,
-					  DBusGMethodInvocation *context)
-{
-	FprintDevicePrivate *priv = DEVICE_GET_PRIVATE(rdev);
-	GHashTable *table;
-	GValue *value;
-
-	table = g_hash_table_new_full (g_str_hash, g_str_equal, NULL, g_free);
-
-	value = g_new0 (GValue, 1);
-	g_value_init (value, G_TYPE_STRING);
-	g_value_set_string (value, fp_driver_get_full_name (fp_dscv_dev_get_driver (priv->ddev)));
-	g_hash_table_insert (table, "Name", value);
-
-	value = g_new0 (GValue, 1);
-	g_value_init (value, G_TYPE_STRING);
-	g_value_set_static_string (value,
-				   fp_driver_get_scan_type (fp_dscv_dev_get_driver (priv->ddev)) == FP_SCAN_TYPE_PRESS ? "press" : "swipe");
-	g_hash_table_insert (table, "ScanType", value);
-
-	if (priv->dev != NULL) {
-		value = g_new0 (GValue, 1);
-		g_value_init (value, G_TYPE_INT);
-		g_value_set_int (value, fp_dev_get_nr_enroll_stages (priv->dev));
-		g_hash_table_insert (table, "NumberEnrollStages", value);
-	}
-
-	dbus_g_method_return (context, table);
-
-	g_hash_table_destroy (table);
-}
-

src/device.xml

@@ -447,10 +447,42 @@
 
 		<!-- ************************************************************ -->
 
-		<method name="GetProperties">
+		<property name="name" type="s" access="read">
-			<arg type="a{sv}" name="properties" direction="out" />
+			<doc:doc>
-			<annotation name="org.freedesktop.DBus.GLib.Async" value="" />
+				<doc:description>
-		</method>
+					<doc:para>
+						The product name of the device.
+					</doc:para>
+				</doc:description>
+			</doc:doc>
+		</property>
+
+		<!-- ************************************************************ -->
+
+		<property name="num-enroll-stages" type="i" access="read">
+			<doc:doc>
+				<doc:description>
+					<doc:para>
+						The number of enrollment stages for the device. This is only available when the device has been claimed, otherwise it will be undefined (-1).
+					</doc:para>
+					<doc:seealso>
+						<doc:ref type="method" to="Device.Claim">Device.Claim</doc:ref> and <doc:ref type="method" to="Device.EnrollStart">Device.EnrollStart</doc:ref>.
+					</doc:seealso>
+				</doc:description>
+			</doc:doc>
+		</property>
+
+		<!-- ************************************************************ -->
+
+		<property name="scan-type" type="s" access="read">
+			<doc:doc>
+				<doc:description>
+					<doc:para>
+						The scan type of the device, either "press" if you place your finger on the device, or "swipe" if you have to swipe your finger.
+					</doc:para>
+				</doc:description>
+			</doc:doc>
+		</property>
 
 	</interface>
 </node>

tests/Makefile.am

@@ -1,14 +1,14 @@
-BUILT_SOURCES = manager-dbus-glue.h device-dbus-glue.h
+BUILT_SOURCES = manager-dbus-glue.h device-dbus-glue.h $(MARSHALFILES)
 noinst_HEADERS = $(BUILT_SOURCES)
 CLEANFILES = $(BUILT_SOURCES)
 
 bin_PROGRAMS = fprintd-verify fprintd-enroll fprintd-list
 
-fprintd_verify_SOURCES = verify.c
+fprintd_verify_SOURCES = verify.c $(MARSHALFILES)
 fprintd_verify_CFLAGS = $(WARN_CFLAGS) $(GLIB_CFLAGS)
 fprintd_verify_LDADD = $(GLIB_LIBS)
 
-fprintd_enroll_SOURCES = enroll.c
+fprintd_enroll_SOURCES = enroll.c $(MARSHALFILES)
 fprintd_enroll_CFLAGS = $(WARN_CFLAGS) $(GLIB_CFLAGS)
 fprintd_enroll_LDADD = $(GLIB_LIBS)
 
@@ -21,3 +21,11 @@ manager-dbus-glue.h: ../src/manager.xml
 
 device-dbus-glue.h: ../src/device.xml
 	dbus-binding-tool --prefix=fprint_device --mode=glib-client $< --output=$@
+
+MARSHALFILES = marshal.c marshal.h
+GLIB_GENMARSHAL=`pkg-config --variable=glib_genmarshal glib-2.0`
+
+marshal.h: $(top_srcdir)/src/fprintd-marshal.list
+	( $(GLIB_GENMARSHAL) --prefix=fprintd_marshal $(top_srcdir)/src/fprintd-marshal.list --header > marshal.h )
+marshal.c: marshal.h
+	( $(GLIB_GENMARSHAL) --prefix=fprintd_marshal $(top_srcdir)/src/fprintd-marshal.list --body --header > marshal.c )

tests/enroll.c

@@ -21,6 +21,7 @@
 #include <dbus/dbus-glib-bindings.h>
 #include "manager-dbus-glue.h"
 #include "device-dbus-glue.h"
+#include "marshal.h"
 
 static DBusGProxy *manager = NULL;
 static DBusGConnection *connection = NULL;
@@ -79,7 +80,7 @@ static void do_enroll(DBusGProxy *dev)
 	GError *error;
 	gboolean enroll_completed = FALSE;
 
-	dbus_g_proxy_add_signal(dev, "EnrollStatus", G_TYPE_INT, NULL);
+	dbus_g_proxy_add_signal(dev, "EnrollStatus", G_TYPE_STRING, G_TYPE_BOOLEAN, NULL);
 	dbus_g_proxy_connect_signal(dev, "EnrollStatus", G_CALLBACK(enroll_result),
 				    &enroll_completed, NULL);
 
@@ -111,6 +112,10 @@ int main(int argc, char **argv)
 	char *username;
 
 	g_type_init();
+
+	dbus_g_object_register_marshaller (fprintd_marshal_VOID__STRING_BOOLEAN,
+					   G_TYPE_NONE, G_TYPE_STRING, G_TYPE_BOOLEAN, G_TYPE_INVALID);
+
 	loop = g_main_loop_new(NULL, FALSE);
 	create_manager();
 

tests/list.c

@@ -44,6 +44,7 @@ static void list_fingerprints(DBusGProxy *dev, const char *username)
 	GError *error = NULL;
 	char **fingers;
 	GHashTable *props;
+	DBusGProxy *p;
 	guint i;
 
 	if (!net_reactivated_Fprint_Device_list_enrolled_fingers(dev, username, &fingers, &error))
@@ -54,14 +55,19 @@ static void list_fingerprints(DBusGProxy *dev, const char *username)
 		return;
 	}
 
-	if (!net_reactivated_Fprint_Device_get_properties(dev, &props, &error))
+	p = dbus_g_proxy_new_for_name(connection,
-		g_error("GetProperties failed: %s", error->message);
+				      "net.reactivated.Fprint", dbus_g_proxy_get_path (dev),
+				      "org.freedesktop.DBus.Properties");
+	if (!dbus_g_proxy_call (p, "GetAll", &error, G_TYPE_STRING, "net.reactivated.Fprint.Device", G_TYPE_INVALID,
+			   dbus_g_type_get_map ("GHashTable", G_TYPE_STRING, G_TYPE_VALUE), &props, G_TYPE_INVALID))
+		g_error("GetAll on the Properties interface failed: %s", error->message);
 
 	g_print("Fingerprints for user %s on %s (%s):\n",
 		username,
-		g_value_get_string (g_hash_table_lookup (props, "Name")),
+		g_value_get_string (g_hash_table_lookup (props, "name")),
-		g_value_get_string (g_hash_table_lookup (props, "ScanType")));
+		g_value_get_string (g_hash_table_lookup (props, "scan-type")));
 	g_hash_table_destroy (props);
+	g_object_unref (p);
 
 	for (i = 0; fingers[i] != NULL; i++) {
 		g_print(" - #%d: %s\n", i, fingers[i]);

tests/verify.c

@@ -23,6 +23,7 @@
 #include <dbus/dbus-glib-bindings.h>
 #include "manager-dbus-glue.h"
 #include "device-dbus-glue.h"
+#include "marshal.h"
 
 static DBusGProxy *manager = NULL;
 static DBusGConnection *connection = NULL;
@@ -114,7 +115,7 @@ static void do_verify(DBusGProxy *dev)
 	GError *error;
 	gboolean verify_completed = FALSE;
 
-	dbus_g_proxy_add_signal(dev, "VerifyStatus", G_TYPE_INT, NULL);
+	dbus_g_proxy_add_signal(dev, "VerifyStatus", G_TYPE_STRING, G_TYPE_BOOLEAN, NULL);
 	dbus_g_proxy_add_signal(dev, "VerifyFingerSelected", G_TYPE_INT, NULL);
 	dbus_g_proxy_connect_signal(dev, "VerifyStatus", G_CALLBACK(verify_result),
 				    &verify_completed, NULL);
@@ -158,6 +159,9 @@ int main(int argc, char **argv)
 
 	g_type_init();
 
+	dbus_g_object_register_marshaller (fprintd_marshal_VOID__STRING_BOOLEAN,
+					   G_TYPE_NONE, G_TYPE_STRING, G_TYPE_BOOLEAN, G_TYPE_INVALID);
+
 	context = g_option_context_new ("Verify a fingerprint");
 	g_option_context_add_main_entries (context, entries, NULL);