From c9fdeb47aa2f2c02c9a5080542aae41ef04e09d9 Mon Sep 17 00:00:00 2001 From: Bastien Nocera Date: Sat, 22 Nov 2008 00:00:06 +0000 Subject: [PATCH] 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 | 4 --- pam/Makefile.am | 11 +++++- pam/pam_fprintd.c | 28 +++++++++++---- src/device.c | 86 ++++++++++++++++++++++++++--------------------- src/device.xml | 40 +++++++++++++++++++--- tests/Makefile.am | 14 ++++++-- tests/enroll.c | 9 +++-- tests/list.c | 14 +++++--- tests/verify.c | 8 +++-- 9 files changed, 148 insertions(+), 66 deletions(-) diff --git a/TODO b/TODO index d276102..3e1fcee 100644 --- a/TODO +++ b/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 diff --git a/pam/Makefile.am b/pam/Makefile.am index d262023..dd6b41d 100644 --- a/pam/Makefile.am +++ b/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 diff --git a/pam/pam_fprintd.c b/pam/pam_fprintd.c index 284a887..4955bfb 100644 --- a/pam/pam_fprintd.c +++ b/pam/pam_fprintd.c @@ -32,6 +32,8 @@ #define PAM_SM_AUTH #include +#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, - dbus_g_type_get_map ("GHashTable", G_TYPE_STRING, G_TYPE_STRING), &props, G_TYPE_INVALID)) { + /* Get some properties for the device */ + 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")); - scan_type = g_value_dup_string (g_hash_table_lookup (props, "ScanType")); + data->driver = g_value_dup_string (g_hash_table_lookup (props, "name")); + 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, "VerifyFingerSelected", 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_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) */ diff --git a/src/device.c b/src/device.c index c2fc5ed..e3f7c3b 100644 --- a/src/device.c +++ b/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; @@ -192,16 +213,35 @@ static void fprint_device_class_init(FprintDeviceClass *klass) g_type_class_add_private(klass, sizeof(FprintDevicePrivate)); pspec = g_param_spec_pointer("discovered-dev", "Discovered device", - "Set discovered device construction property", - G_PARAM_CONSTRUCT_ONLY | G_PARAM_WRITABLE); + "Set discovered device construction property", + G_PARAM_CONSTRUCT_ONLY | G_PARAM_WRITABLE); g_object_class_install_property(gobject_class, - FPRINT_DEVICE_CONSTRUCT_DDEV, pspec); + 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); + "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); -} - diff --git a/src/device.xml b/src/device.xml index a7ebfba..6e595f2 100644 --- a/src/device.xml +++ b/src/device.xml @@ -447,10 +447,42 @@ - - - - + + + + + The product name of the device. + + + + + + + + + + + + The number of enrollment stages for the device. This is only available when the device has been claimed, otherwise it will be undefined (-1). + + + Device.Claim and Device.EnrollStart. + + + + + + + + + + + + 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. + + + + diff --git a/tests/Makefile.am b/tests/Makefile.am index 7cb0b92..c0140dc 100644 --- a/tests/Makefile.am +++ b/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 ) diff --git a/tests/enroll.c b/tests/enroll.c index 431456f..c8b5b40 100644 --- a/tests/enroll.c +++ b/tests/enroll.c @@ -21,6 +21,7 @@ #include #include "manager-dbus-glue.h" #include "device-dbus-glue.h" +#include "marshal.h" static DBusGProxy *manager = NULL; static DBusGConnection *connection = NULL; @@ -79,9 +80,9 @@ 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); + &enroll_completed, NULL); g_print("Enrolling right index finger.\n"); if (!net_reactivated_Fprint_Device_enroll_start(dev, "right-index-finger", &error)) @@ -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(); diff --git a/tests/list.c b/tests/list.c index 41d278f..1e9fac5 100644 --- a/tests/list.c +++ b/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)) - g_error("GetProperties failed: %s", error->message); + 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_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, "ScanType"))); + g_value_get_string (g_hash_table_lookup (props, "name")), + 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]); diff --git a/tests/verify.c b/tests/verify.c index 3dbb2c8..999c402 100644 --- a/tests/verify.c +++ b/tests/verify.c @@ -23,6 +23,7 @@ #include #include "manager-dbus-glue.h" #include "device-dbus-glue.h" +#include "marshal.h" static DBusGProxy *manager = NULL; static DBusGConnection *connection = NULL; @@ -114,10 +115,10 @@ 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); + &verify_completed, NULL); dbus_g_proxy_connect_signal(dev, "VerifyFingerSelected", G_CALLBACK(verify_finger_selected), NULL, 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);