From 7b06c4b7f3b518786137f8b162b402107cbe2e72 Mon Sep 17 00:00:00 2001 From: Bastien Nocera Date: Mon, 24 Nov 2008 14:16:41 +0000 Subject: [PATCH] Review possible errors and document them Review all the possible errors and document them for each function. --- TODO | 6 ---- src/device.c | 36 +++++++++++----------- src/device.xml | 74 ++++++++++++++++++++++++++++++++++++++++++---- src/file_storage.c | 5 ---- src/fprintd.h | 19 +++++------- src/manager.c | 17 ++++------- 6 files changed, 100 insertions(+), 57 deletions(-) diff --git a/TODO b/TODO index 3e1fcee..e95a083 100644 --- a/TODO +++ b/TODO @@ -1,10 +1,6 @@ Identification Image transfer -Enforce command sequences: - force VerifyStop after non-retry verify result comes in - etc - Verify PAM messages fit with GDM/gnome-screensaver Register fprintd' po file with Transifex, Rosetta or the Translation Project @@ -19,5 +15,3 @@ reading for more than a certain amount of time. Add POS use case -Review error cases, and possible errors - diff --git a/src/device.c b/src/device.c index e3f7c3b..9034a75 100644 --- a/src/device.c +++ b/src/device.c @@ -466,7 +466,7 @@ _fprint_device_check_polkit_for_action (FprintDevice *rdev, DBusGMethodInvocatio if (pk_result != POLKIT_RESULT_YES) { g_set_error (error, FPRINT_ERROR, - FPRINT_ERROR_INTERNAL, + FPRINT_ERROR_PERMISSION_DENIED, "%s %s <-- (action, result)", action, polkit_result_to_string_representation (pk_result)); @@ -522,7 +522,7 @@ _fprint_device_check_for_username (FprintDevice *rdev, user = getpwuid (uid); if (user == NULL) { g_free (sender); - g_set_error(error, FPRINT_ERROR, FPRINT_ERROR_CLAIM_DEVICE, + g_set_error(error, FPRINT_ERROR, FPRINT_ERROR_INTERNAL, "Failed to get information about user UID %lu", uid); return NULL; } @@ -644,7 +644,7 @@ static void dev_open_cb(struct fp_dev *dev, int status, void *user_data) g_free (priv->sender); priv->sender = NULL; - g_set_error(&error, FPRINT_ERROR, FPRINT_ERROR_CLAIM_DEVICE, + g_set_error(&error, FPRINT_ERROR, FPRINT_ERROR_INTERNAL, "Open failed with error %d", status); dbus_g_method_return_error(session->context_claim_device, error); return; @@ -665,7 +665,7 @@ static void fprint_device_claim(FprintDevice *rdev, /* Is it already claimed? */ if (priv->sender != NULL) { - g_set_error(&error, FPRINT_ERROR, FPRINT_ERROR_CLAIM_DEVICE, + g_set_error(&error, FPRINT_ERROR, FPRINT_ERROR_ALREADY_IN_USE, "Device was already claimed"); dbus_g_method_return_error(context, error); return; @@ -717,7 +717,7 @@ static void fprint_device_claim(FprintDevice *rdev, g_free (priv->sender); priv->sender = NULL; - g_set_error(&error, FPRINT_ERROR, FPRINT_ERROR_CLAIM_DEVICE, + g_set_error(&error, FPRINT_ERROR, FPRINT_ERROR_INTERNAL, "Could not attempt device open, error %d", r); dbus_g_method_return_error(context, error); } @@ -839,7 +839,7 @@ static void fprint_device_verify_start(FprintDevice *rdev, prints = store.discover_prints(priv->ddev, priv->username); if (prints == NULL) { - g_set_error(&error, FPRINT_ERROR, FPRINT_ERROR_NO_SUCH_LOADED_PRINT, + g_set_error(&error, FPRINT_ERROR, FPRINT_ERROR_NO_ENROLLED_PRINTS, "No fingerprints enrolled"); dbus_g_method_return_error(context, error); return; @@ -854,8 +854,8 @@ static void fprint_device_verify_start(FprintDevice *rdev, g_message ("adding finger %d to the gallery", GPOINTER_TO_INT (l->data)); r = store.print_data_load(priv->dev, GPOINTER_TO_INT (l->data), &data, priv->username); - //FIXME r < 0 ? - g_ptr_array_add (array, data); + if (r == 0) + g_ptr_array_add (array, data); } data = NULL; @@ -873,7 +873,7 @@ static void fprint_device_verify_start(FprintDevice *rdev, if (fp_dev_supports_identification(priv->dev) && finger_num == -1) { if (gallery == NULL) { - g_set_error(&error, FPRINT_ERROR, FPRINT_ERROR_NO_SUCH_LOADED_PRINT, + g_set_error(&error, FPRINT_ERROR, FPRINT_ERROR_NO_ENROLLED_PRINTS, "No fingerprints on that device"); dbus_g_method_return_error(context, error); g_error_free (error); @@ -893,7 +893,7 @@ static void fprint_device_verify_start(FprintDevice *rdev, &data, priv->username); if (r < 0 || !data) { - g_set_error(&error, FPRINT_ERROR, FPRINT_ERROR_NO_SUCH_LOADED_PRINT, + g_set_error(&error, FPRINT_ERROR, FPRINT_ERROR_INTERNAL, "No such print %d", finger_num); dbus_g_method_return_error(context, error); return; @@ -918,7 +918,7 @@ static void fprint_device_verify_start(FprintDevice *rdev, fp_print_data_free(gallery[i]); g_free (gallery); } - g_set_error(&error, FPRINT_ERROR, FPRINT_ERROR_VERIFY_START, + g_set_error(&error, FPRINT_ERROR, FPRINT_ERROR_INTERNAL, "Verify start failed with error %d", r); dbus_g_method_return_error(context, error); return; @@ -959,7 +959,7 @@ static void fprint_device_verify_stop(FprintDevice *rdev, } else if (priv->current_action == ACTION_IDENTIFY) { r = fp_async_identify_stop(priv->dev, identify_stop_cb, context); } else { - g_set_error(&error, FPRINT_ERROR, FPRINT_ERROR_VERIFY_STOP, + g_set_error(&error, FPRINT_ERROR, FPRINT_ERROR_NO_ACTION_IN_PROGRESS, "No verification in progress"); dbus_g_method_return_error(context, error); g_error_free (error); @@ -967,7 +967,7 @@ static void fprint_device_verify_stop(FprintDevice *rdev, } if (r < 0) { - g_set_error(&error, FPRINT_ERROR, FPRINT_ERROR_VERIFY_STOP, + g_set_error(&error, FPRINT_ERROR, FPRINT_ERROR_INTERNAL, "Verify stop failed with error %d", r); dbus_g_method_return_error(context, error); g_error_free (error); @@ -1009,7 +1009,7 @@ static void fprint_device_enroll_start(FprintDevice *rdev, int r; if (finger_num == -1) { - g_set_error(&error, FPRINT_ERROR, FPRINT_ERROR_NO_SUCH_LOADED_PRINT, + g_set_error(&error, FPRINT_ERROR, FPRINT_ERROR_INVALID_FINGERNAME, "Invalid print name"); dbus_g_method_return_error(context, error); g_error_free (error); @@ -1044,7 +1044,7 @@ static void fprint_device_enroll_start(FprintDevice *rdev, r = fp_async_enroll_start(priv->dev, enroll_stage_cb, rdev); if (r < 0) { - g_set_error(&error, FPRINT_ERROR, FPRINT_ERROR_ENROLL_START, + g_set_error(&error, FPRINT_ERROR, FPRINT_ERROR_INTERNAL, "Enroll start failed with error %d", r); dbus_g_method_return_error(context, error); return; @@ -1078,7 +1078,7 @@ static void fprint_device_enroll_stop(FprintDevice *rdev, } if (priv->current_action != ACTION_ENROLL) { - g_set_error(&error, FPRINT_ERROR, FPRINT_ERROR_ENROLL_STOP, + g_set_error(&error, FPRINT_ERROR, FPRINT_ERROR_NO_ACTION_IN_PROGRESS, "No enrollment in progress"); dbus_g_method_return_error(context, error); g_error_free (error); @@ -1087,7 +1087,7 @@ static void fprint_device_enroll_stop(FprintDevice *rdev, r = fp_async_enroll_stop(priv->dev, enroll_stop_cb, context); if (r < 0) { - g_set_error(&error, FPRINT_ERROR, FPRINT_ERROR_ENROLL_STOP, + g_set_error(&error, FPRINT_ERROR, FPRINT_ERROR_INTERNAL, "Enroll stop failed with error %d", r); dbus_g_method_return_error(context, error); g_error_free (error); @@ -1131,7 +1131,7 @@ static void fprint_device_list_enrolled_fingers(FprintDevice *rdev, prints = store.discover_prints(priv->ddev, user); g_free (user); if (!prints) { - g_set_error(&error, FPRINT_ERROR, FPRINT_ERROR_DISCOVER_PRINTS, + g_set_error(&error, FPRINT_ERROR, FPRINT_ERROR_NO_ENROLLED_PRINTS, "Failed to discover prints"); dbus_g_method_return_error(context, error); return; diff --git a/src/device.xml b/src/device.xml index 6e595f2..402c44b 100644 --- a/src/device.xml +++ b/src/device.xml @@ -1,4 +1,15 @@ - + + + + + + + +]> + + + + if the caller lacks the appropriate PolicyKit authorization + if the chosen user doesn't have any fingerprints enrolled + @@ -273,6 +289,10 @@ Delete all the enrolled fingerprints for the chosen user. + + + if the caller lacks the appropriate PolicyKit authorization + @@ -290,6 +310,12 @@ Claim the device for the chosen user. + + + if the caller lacks the appropriate PolicyKit authorization + if the device is already claimed + if the device couldn't be claimed + @@ -304,6 +330,11 @@ Release a device claimed with Device.Claim. + + + if the caller lacks the appropriate PolicyKit authorization + if the device was not claimed + @@ -319,9 +350,19 @@ Check the chosen finger against a saved fingerprint. You need to have claimed the device using - Device.Claim. + Device.Claim. The finger selected is sent to the front-end + using Device::VerifyFingerSelected and + verification status through Device::VerifyStatus. + + + if the caller lacks the appropriate PolicyKit authorization + if the device was not claimed + if the device was already being used + if there are no enrolled prints for the chosen user + if there was an internal error + @@ -336,6 +377,13 @@ Stop an on-going fingerprint verification started with Device.VerifyStart. + + + if the caller lacks the appropriate PolicyKit authorization + if the device was not claimed + if there was no ongoing verification + if there was an internal error + @@ -364,7 +412,7 @@ - A string representing the status of the verification status. + A string representing the status of the verification. @@ -399,9 +447,18 @@ Start enrollemnt for the selected finger. You need to have claimed the device using Device.Claim before calling - this method. + this method. Enrollment status is sent through Device::EnrollStatus. + + + if the caller lacks the appropriate PolicyKit authorization + if the device was not claimed + if the device was already being used + if there are no enrolled prints for the chosen user + if there was an internal error + + @@ -416,6 +473,13 @@ Stop an on-going fingerprint enrollment started with Device.EnrollStart. + + + if the caller lacks the appropriate PolicyKit authorization + if the device was not claimed + if there was no ongoing verification + if there was an internal error + @@ -425,7 +489,7 @@ - A string representing the status of the enrollment status. + A string representing the status of the enrollment. diff --git a/src/file_storage.c b/src/file_storage.c index 7180b8a..78b53af 100644 --- a/src/file_storage.c +++ b/src/file_storage.c @@ -257,11 +257,7 @@ static GSList *scan_dev_storedir(char *devpath, uint16_t driver_id, GSList *file_storage_discover_prints(struct fp_dscv_dev *dev, const char *username) { - //GDir *dir; - //const gchar *ent; - //GError *err = NULL; GSList *list = NULL; - //GSList *elem; char *base_store = NULL; char *storedir = NULL; int r; @@ -276,7 +272,6 @@ GSList *file_storage_discover_prints(struct fp_dscv_dev *dev, const char *userna storedir = get_path_to_storedir(fp_driver_get_driver_id(fp_dscv_dev_get_driver(dev)), fp_dscv_dev_get_devtype(dev), base_store); - g_message("Entering %s", storedir); list = scan_dev_storedir(storedir, fp_driver_get_driver_id(fp_dscv_dev_get_driver(dev)), fp_dscv_dev_get_devtype(dev), list); diff --git a/src/fprintd.h b/src/fprintd.h index 901e92d..2006f4b 100644 --- a/src/fprintd.h +++ b/src/fprintd.h @@ -36,18 +36,13 @@ GType fprint_error_get_type(void); #define FPRINT_TYPE_ERROR fprint_error_get_type() #define FPRINT_ERROR_DBUS_INTERFACE "net.reactivated.Fprint.Error" typedef enum { - FPRINT_ERROR_INTERNAL, - FPRINT_ERROR_ALREADY_IN_USE, - FPRINT_ERROR_DISCOVER_PRINTS, - FPRINT_ERROR_PRINT_NOT_FOUND, - FPRINT_ERROR_PRINT_LOAD, - FPRINT_ERROR_NO_SUCH_LOADED_PRINT, - FPRINT_ERROR_CLAIM_DEVICE, - FPRINT_ERROR_VERIFY_START, - FPRINT_ERROR_VERIFY_STOP, - FPRINT_ERROR_ENROLL_START, - FPRINT_ERROR_ENROLL_STOP, - FPRINT_ERROR_FAILED, + FPRINT_ERROR_CLAIM_DEVICE, /* developer didn't claim the device */ + FPRINT_ERROR_ALREADY_IN_USE, /* device is already claimed by somebody else */ + FPRINT_ERROR_INTERNAL, /* internal error occured */ + FPRINT_ERROR_PERMISSION_DENIED, /* PolicyKit refused the action */ + FPRINT_ERROR_NO_ENROLLED_PRINTS, /* No prints are enrolled */ + FPRINT_ERROR_NO_ACTION_IN_PROGRESS, /* No actions currently in progress */ + FPRINT_ERROR_INVALID_FINGERNAME, /* the finger name passed was invalid */ } FprintError; /* Manager */ diff --git a/src/manager.c b/src/manager.c index 3a590d2..4c88ed2 100644 --- a/src/manager.c +++ b/src/manager.c @@ -207,18 +207,13 @@ fprint_error_get_type (void) if (etype == 0) { static const GEnumValue values[] = { - ENUM_ENTRY (FPRINT_ERROR_INTERNAL, "Internal"), - ENUM_ENTRY (FPRINT_ERROR_ALREADY_IN_USE, "InUse"), - ENUM_ENTRY (FPRINT_ERROR_DISCOVER_PRINTS, "DiscoverPrints"), - ENUM_ENTRY (FPRINT_ERROR_PRINT_NOT_FOUND, "PrintNotFound"), - ENUM_ENTRY (FPRINT_ERROR_PRINT_LOAD, "PrintLoad"), - ENUM_ENTRY (FPRINT_ERROR_NO_SUCH_LOADED_PRINT, "NoSuchLoadedPrint"), ENUM_ENTRY (FPRINT_ERROR_CLAIM_DEVICE, "ClaimDevice"), - ENUM_ENTRY (FPRINT_ERROR_VERIFY_START, "VerifyStart"), - ENUM_ENTRY (FPRINT_ERROR_VERIFY_STOP, "VerifyStop"), - ENUM_ENTRY (FPRINT_ERROR_ENROLL_START, "EnrollStart"), - ENUM_ENTRY (FPRINT_ERROR_ENROLL_STOP, "EnrollStop"), - ENUM_ENTRY (FPRINT_ERROR_FAILED, "Failed"), + ENUM_ENTRY (FPRINT_ERROR_ALREADY_IN_USE, "AlreadyInUse"), + ENUM_ENTRY (FPRINT_ERROR_INTERNAL, "Internal"), + ENUM_ENTRY (FPRINT_ERROR_PERMISSION_DENIED, "PermissionDenied"), + ENUM_ENTRY (FPRINT_ERROR_NO_ENROLLED_PRINTS, "NoEnrolledPrints"), + ENUM_ENTRY (FPRINT_ERROR_NO_ACTION_IN_PROGRESS, "NoActionInProgress"), + ENUM_ENTRY (FPRINT_ERROR_INVALID_FINGERNAME, "InvalidFingername"), { 0, 0, 0 } }; etype = g_enum_register_static ("FprintError", values);