From abb0b1267cc135e6445d882729136a577daf1700 Mon Sep 17 00:00:00 2001 From: Aris Lin Date: Wed, 2 Dec 2020 10:21:39 +0800 Subject: [PATCH 01/22] synaptics: add support PID 0xE7 --- libfprint/drivers/synaptics/synaptics.c | 1 + 1 file changed, 1 insertion(+) diff --git a/libfprint/drivers/synaptics/synaptics.c b/libfprint/drivers/synaptics/synaptics.c index 5d824a6c..934a271e 100644 --- a/libfprint/drivers/synaptics/synaptics.c +++ b/libfprint/drivers/synaptics/synaptics.c @@ -35,6 +35,7 @@ static const FpIdEntry id_table[] = { { .vid = SYNAPTICS_VENDOR_ID, .pid = 0xFC, }, { .vid = SYNAPTICS_VENDOR_ID, .pid = 0xC2, }, { .vid = SYNAPTICS_VENDOR_ID, .pid = 0xC9, }, + { .vid = SYNAPTICS_VENDOR_ID, .pid = 0xE7, }, { .vid = 0, .pid = 0, .driver_data = 0 }, /* terminating entry */ }; From 2783ac3e60858a8c9d2d910ae80889712497fc89 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Wed, 2 Dec 2020 17:05:01 +0100 Subject: [PATCH 02/22] fpi-device: Return proper type on identification success Identify function is supposed to propagate a boolean value, but we make it return an integer instead on idle, this can be normally the same in most of architectures, but not in BE ones. So, make it return the proper type. Fixes test failures in s390x. Related to #236 --- libfprint/fpi-device.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libfprint/fpi-device.c b/libfprint/fpi-device.c index 981df878..0968c0c1 100644 --- a/libfprint/fpi-device.c +++ b/libfprint/fpi-device.c @@ -1157,7 +1157,7 @@ fpi_device_identify_complete (FpDevice *device, } else { - fpi_device_return_task_in_idle (device, FP_DEVICE_TASK_RETURN_INT, GINT_TO_POINTER (TRUE)); + fpi_device_return_task_in_idle (device, FP_DEVICE_TASK_RETURN_BOOL, GUINT_TO_POINTER (TRUE)); } } else From 12b0120a3de5f1b64f356b2813a12edb657e0325 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Wed, 2 Dec 2020 17:27:00 +0100 Subject: [PATCH 03/22] test-fpi-device: Always check the return values for the API calls Ensure that the return value of the API calls match the expected one, as we need to ensure that it also matches with the error/no-error case. --- tests/test-fpi-device.c | 121 ++++++++++++++++++++-------------------- 1 file changed, 62 insertions(+), 59 deletions(-) diff --git a/tests/test-fpi-device.c b/tests/test-fpi-device.c index 43e84f79..d492bc7a 100644 --- a/tests/test-fpi-device.c +++ b/tests/test-fpi-device.c @@ -534,12 +534,12 @@ test_driver_open (void) g_assert (fake_dev->last_called_function != dev_class->probe); - fp_device_open_sync (device, NULL, &error); + g_assert_true (fp_device_open_sync (device, NULL, &error)); g_assert (fake_dev->last_called_function == dev_class->open); g_assert_no_error (error); g_assert_true (fp_device_is_open (device)); - fp_device_close_sync (FP_DEVICE (device), NULL, &error); + g_assert_true (fp_device_close_sync (FP_DEVICE (device), NULL, &error)); g_assert_no_error (error); } @@ -552,7 +552,7 @@ test_driver_open_error (void) FpiDeviceFake *fake_dev = FPI_DEVICE_FAKE (device); fake_dev->ret_error = fpi_device_error_new (FP_DEVICE_ERROR_GENERAL); - fp_device_open_sync (device, NULL, &error); + g_assert_false (fp_device_open_sync (device, NULL, &error)); g_assert (fake_dev->last_called_function == dev_class->open); g_assert_error (error, FP_DEVICE_ERROR, FP_DEVICE_ERROR_GENERAL); g_assert_false (fp_device_is_open (device)); @@ -567,7 +567,7 @@ test_driver_close (void) FpDeviceClass *dev_class = FP_DEVICE_GET_CLASS (device); FpiDeviceFake *fake_dev = FPI_DEVICE_FAKE (device); - fp_device_close_sync (device, NULL, &error); + g_assert_true (fp_device_close_sync (device, NULL, &error)); g_assert (fake_dev->last_called_function == dev_class->close); g_assert_no_error (error); @@ -583,7 +583,7 @@ test_driver_close_error (void) FpiDeviceFake *fake_dev = FPI_DEVICE_FAKE (device); fake_dev->ret_error = fpi_device_error_new (FP_DEVICE_ERROR_GENERAL); - fp_device_close_sync (device, NULL, &error); + g_assert_false (fp_device_close_sync (device, NULL, &error)); g_assert (fake_dev->last_called_function == dev_class->close); g_assert_error (error, FP_DEVICE_ERROR, FP_DEVICE_ERROR_GENERAL); @@ -880,9 +880,9 @@ test_driver_verify (void) gboolean match; fake_dev->ret_result = FPI_MATCH_SUCCESS; - fp_device_verify_sync (device, enrolled_print, NULL, - test_driver_match_cb, match_data, - &match, &out_print, &error); + g_assert_true (fp_device_verify_sync (device, enrolled_print, NULL, + test_driver_match_cb, match_data, + &match, &out_print, &error)); g_assert (fake_dev->last_called_function == dev_class->verify); g_assert (fake_dev->action_data == enrolled_print); @@ -910,9 +910,9 @@ test_driver_verify_fail (void) gboolean match; fake_dev->ret_result = FPI_MATCH_FAIL; - fp_device_verify_sync (device, enrolled_print, NULL, - test_driver_match_cb, match_data, - &match, &out_print, &error); + g_assert_true (fp_device_verify_sync (device, enrolled_print, NULL, + test_driver_match_cb, match_data, + &match, &out_print, &error)); g_assert (fake_dev->last_called_function == dev_class->verify); g_assert_no_error (error); @@ -940,9 +940,9 @@ test_driver_verify_retry (void) fake_dev->ret_result = FPI_MATCH_ERROR; fake_dev->ret_error = fpi_device_retry_new (FP_DEVICE_RETRY_GENERAL); - fp_device_verify_sync (device, enrolled_print, NULL, - test_driver_match_cb, match_data, - &match, &out_print, &error); + g_assert_false (fp_device_verify_sync (device, enrolled_print, NULL, + test_driver_match_cb, match_data, + &match, &out_print, &error)); g_assert_true (match_data->called); g_assert_null (match_data->match); @@ -968,9 +968,9 @@ test_driver_verify_error (void) fake_dev->ret_result = FPI_MATCH_ERROR; fake_dev->ret_error = fpi_device_error_new (FP_DEVICE_ERROR_GENERAL); - fp_device_verify_sync (device, enrolled_print, NULL, - test_driver_match_cb, match_data, - &match, &out_print, &error); + g_assert_false (fp_device_verify_sync (device, enrolled_print, NULL, + test_driver_match_cb, match_data, + &match, &out_print, &error)); g_assert_false (match_data->called); g_assert_null (match_data->match); @@ -1005,9 +1005,9 @@ test_driver_verify_not_reported (void) g_test_expect_message (G_LOG_DOMAIN, G_LOG_LEVEL_WARNING, "*reported successful verify complete*not report*result*"); - fp_device_verify_sync (device, enrolled_print, NULL, - NULL, NULL, - NULL, NULL, &error); + g_assert_false (fp_device_verify_sync (device, enrolled_print, NULL, + NULL, NULL, + NULL, NULL, &error)); g_assert_error (error, FP_DEVICE_ERROR, FP_DEVICE_ERROR_GENERAL); @@ -1050,9 +1050,9 @@ test_driver_verify_report_no_callback (void) fake_dev->ret_result = FPI_MATCH_ERROR; fake_dev->ret_error = fpi_device_error_new (FP_DEVICE_ERROR_NOT_SUPPORTED); - fp_device_verify_sync (device, enrolled_print, NULL, - test_driver_match_cb, match_data, - &match, &print, &error); + g_assert_false (fp_device_verify_sync (device, enrolled_print, NULL, + test_driver_match_cb, match_data, + &match, &print, &error)); g_test_assert_expected_messages (); @@ -1091,8 +1091,8 @@ test_driver_verify_complete_retry (void) test_driver_match_data_clear (match_data); fake_dev->ret_result = FPI_MATCH_FAIL; fake_dev->ret_error = fpi_device_retry_new (FP_DEVICE_RETRY_TOO_SHORT); - fp_device_verify_sync (device, enrolled_print, NULL, test_driver_match_cb, - match_data, &match, &print, &error); + g_assert_false (fp_device_verify_sync (device, enrolled_print, NULL, test_driver_match_cb, + match_data, &match, &print, &error)); g_test_assert_expected_messages (); g_assert_true (error == g_steal_pointer (&fake_dev->ret_error)); @@ -1113,8 +1113,8 @@ test_driver_verify_complete_retry (void) fake_dev->ret_result = FPI_MATCH_FAIL; fake_dev->ret_error = fpi_device_retry_new (FP_DEVICE_RETRY_TOO_SHORT); fake_dev->user_data = g_error_copy (fake_dev->ret_error); - fp_device_verify_sync (device, enrolled_print, NULL, test_driver_match_cb, - match_data, &match, &print, &error); + g_assert_false (fp_device_verify_sync (device, enrolled_print, NULL, test_driver_match_cb, + match_data, &match, &print, &error)); g_test_assert_expected_messages (); g_assert_true (error != g_steal_pointer (&fake_dev->ret_error)); @@ -1135,8 +1135,8 @@ test_driver_verify_complete_retry (void) fake_dev->ret_error = fpi_device_retry_new (FP_DEVICE_RETRY_TOO_SHORT); fake_dev->user_data = g_error_copy (fake_dev->ret_error); - fp_device_verify_sync (device, enrolled_print, NULL, test_driver_match_cb, - match_data, &match, &print, &error); + g_assert_false (fp_device_verify_sync (device, enrolled_print, NULL, test_driver_match_cb, + match_data, &match, &print, &error)); g_test_assert_expected_messages (); g_assert_true (error != g_steal_pointer (&fake_dev->ret_error)); @@ -1155,8 +1155,8 @@ test_driver_verify_complete_retry (void) test_driver_match_data_clear (match_data); fake_dev->ret_result = FPI_MATCH_ERROR; - fp_device_verify_sync (device, enrolled_print, NULL, test_driver_match_cb, - match_data, &match, &print, &error); + g_assert_false (fp_device_verify_sync (device, enrolled_print, NULL, test_driver_match_cb, + match_data, &match, &print, &error)); g_test_assert_expected_messages (); g_assert_true (error != g_steal_pointer (&fake_dev->ret_error)); @@ -1178,8 +1178,8 @@ test_driver_verify_complete_retry (void) g_object_add_weak_pointer (G_OBJECT (fake_dev->ret_print), (gpointer) (&fake_dev->ret_print)); - fp_device_verify_sync (device, enrolled_print, NULL, test_driver_match_cb, - match_data, &match, &print, &error); + g_assert_false (fp_device_verify_sync (device, enrolled_print, NULL, test_driver_match_cb, + match_data, &match, &print, &error)); g_test_assert_expected_messages (); g_assert_error (error, FP_DEVICE_RETRY, FP_DEVICE_RETRY_TOO_SHORT); @@ -1244,9 +1244,9 @@ test_driver_identify (void) g_assert_true (fp_device_supports_identify (device)); fake_dev->ret_print = fp_print_new (device); - fp_device_identify_sync (device, prints, NULL, - test_driver_match_cb, match_data, - &matched_print, &print, &error); + g_assert_true (fp_device_identify_sync (device, prints, NULL, + test_driver_match_cb, match_data, + &matched_print, &print, &error)); g_assert_true (match_data->called); g_assert_nonnull (match_data->match); @@ -1280,9 +1280,9 @@ test_driver_identify_fail (void) g_assert_true (fp_device_supports_identify (device)); fake_dev->ret_print = fp_print_new (device); - fp_device_identify_sync (device, prints, NULL, - test_driver_match_cb, match_data, - &matched_print, &print, &error); + g_assert_true (fp_device_identify_sync (device, prints, NULL, + test_driver_match_cb, match_data, + &matched_print, &print, &error)); g_assert_true (match_data->called); g_assert_null (match_data->match); @@ -1320,9 +1320,9 @@ test_driver_identify_retry (void) g_assert_true (fp_device_supports_identify (device)); fake_dev->ret_error = fpi_device_retry_new (FP_DEVICE_RETRY_GENERAL); - fp_device_identify_sync (device, prints, NULL, - test_driver_match_cb, match_data, - &matched_print, &print, &error); + g_assert_false (fp_device_identify_sync (device, prints, NULL, + test_driver_match_cb, match_data, + &matched_print, &print, &error)); g_assert_true (match_data->called); g_assert_null (match_data->match); @@ -1358,9 +1358,9 @@ test_driver_identify_error (void) g_assert_true (fp_device_supports_identify (device)); fake_dev->ret_error = fpi_device_error_new (FP_DEVICE_ERROR_GENERAL); - fp_device_identify_sync (device, prints, NULL, - test_driver_match_cb, match_data, - &matched_print, &print, &error); + g_assert_false (fp_device_identify_sync (device, prints, NULL, + test_driver_match_cb, match_data, + &matched_print, &print, &error)); g_assert_false (match_data->called); g_assert_null (match_data->match); @@ -1399,9 +1399,9 @@ test_driver_identify_not_reported (void) g_test_expect_message (G_LOG_DOMAIN, G_LOG_LEVEL_WARNING, "*reported successful identify complete*not report*result*"); - fp_device_identify_sync (device, prints, NULL, - NULL, NULL, - NULL, NULL, &error); + g_assert_false (fp_device_identify_sync (device, prints, NULL, + NULL, NULL, + NULL, NULL, &error)); g_assert_error (error, FP_DEVICE_ERROR, FP_DEVICE_ERROR_GENERAL); @@ -1450,8 +1450,9 @@ test_driver_identify_complete_retry (void) fake_dev->ret_error = fpi_device_retry_new (FP_DEVICE_RETRY_TOO_SHORT); fake_dev->user_data = g_error_copy (fake_dev->ret_error); - fp_device_identify_sync (device, prints, NULL, test_driver_match_cb, match_data, - &match, &print, &error); + g_assert_false (fp_device_identify_sync (device, prints, NULL, + test_driver_match_cb, match_data, + &match, &print, &error)); g_test_assert_expected_messages (); g_assert_true (error != g_steal_pointer (&fake_dev->ret_error)); @@ -1470,8 +1471,9 @@ test_driver_identify_complete_retry (void) fake_dev->ret_match = fp_print_new (device); g_object_add_weak_pointer (G_OBJECT (fake_dev->ret_match), (gpointer) (&fake_dev->ret_match)); - fp_device_identify_sync (device, prints, NULL, test_driver_match_cb, match_data, - &match, &print, &error); + g_assert_true (fp_device_identify_sync (device, prints, NULL, + test_driver_match_cb, match_data, + &match, &print, &error)); g_test_assert_expected_messages (); g_object_unref (fake_dev->ret_match); @@ -1493,8 +1495,9 @@ test_driver_identify_complete_retry (void) fake_dev->ret_print = fp_print_new (device); g_object_add_weak_pointer (G_OBJECT (fake_dev->ret_print), (gpointer) (&fake_dev->ret_print)); - fp_device_identify_sync (device, prints, NULL, test_driver_match_cb, match_data, - &match, &print, &error); + g_assert_false (fp_device_identify_sync (device, prints, NULL, + test_driver_match_cb, match_data, + &match, &print, &error)); g_test_assert_expected_messages (); g_assert_error (error, FP_DEVICE_RETRY, FP_DEVICE_RETRY_REMOVE_FINGER); @@ -1531,9 +1534,9 @@ test_driver_identify_report_no_callback (void) "*Driver reported a verify error that was not in the retry domain*"); fake_dev->ret_error = fpi_device_error_new (FP_DEVICE_ERROR_NOT_SUPPORTED); - fp_device_identify_sync (device, prints, NULL, - test_driver_match_cb, match_data, - &match, &print, &error); + g_assert_false (fp_device_identify_sync (device, prints, NULL, + test_driver_match_cb, match_data, + &match, &print, &error)); g_test_assert_expected_messages (); @@ -1818,7 +1821,7 @@ test_driver_cancel_fail (void) FpDeviceClass *dev_class = FP_DEVICE_GET_CLASS (device); FpiDeviceFake *fake_dev = FPI_DEVICE_FAKE (device); - fp_device_delete_print_sync (device, enrolled_print, cancellable, &error); + g_assert_true (fp_device_delete_print_sync (device, enrolled_print, cancellable, &error)); g_assert (fake_dev->last_called_function == dev_class->delete); g_cancellable_cancel (cancellable); @@ -1889,7 +1892,7 @@ test_driver_action_get_cancellable_open (void) fake_dev = FPI_DEVICE_FAKE (device); cancellable = g_cancellable_new (); - fp_device_open_sync (device, cancellable, NULL); + g_assert_true (fp_device_open_sync (device, cancellable, NULL)); g_assert (fake_dev->last_called_function == test_driver_action_get_cancellable_open_vfunc); } From de271a0e8d00c22dc9aed7a37ab4c9eb3abfd5d0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Wed, 2 Dec 2020 14:58:09 +0000 Subject: [PATCH 04/22] fp-print: Don't byte-swap two times the NBIS array contents When serializing an image print in big endian machine we ended up swapping the arrays contents two times, first when adding the values and eventually when calling g_variant_byteswap which already handles this properly. With this, we get the test passing into s390x. Fixes: #236 --- libfprint/fp-print.c | 23 ++++++----------------- 1 file changed, 6 insertions(+), 17 deletions(-) diff --git a/libfprint/fp-print.c b/libfprint/fp-print.c index c485975e..70775c93 100644 --- a/libfprint/fp-print.c +++ b/libfprint/fp-print.c @@ -667,36 +667,25 @@ fp_print_serialize (FpPrint *print, for (i = 0; i < print->prints->len; i++) { struct xyt_struct *xyt = g_ptr_array_index (print->prints, i); - gint j; - gint32 *col = g_new (gint32, xyt->nrows); g_variant_builder_open (&nested, G_VARIANT_TYPE ("(aiaiai)")); - for (j = 0; j < xyt->nrows; j++) - col[j] = GINT32_TO_LE (xyt->xcol[j]); g_variant_builder_add_value (&nested, g_variant_new_fixed_array (G_VARIANT_TYPE_INT32, - col, + xyt->xcol, xyt->nrows, - sizeof (col[0]))); - - for (j = 0; j < xyt->nrows; j++) - col[j] = GINT32_TO_LE (xyt->ycol[j]); + sizeof (xyt->xcol[0]))); g_variant_builder_add_value (&nested, g_variant_new_fixed_array (G_VARIANT_TYPE_INT32, - col, + xyt->ycol, xyt->nrows, - sizeof (col[0]))); - - for (j = 0; j < xyt->nrows; j++) - col[j] = GINT32_TO_LE (xyt->thetacol[j]); + sizeof (xyt->ycol[0]))); g_variant_builder_add_value (&nested, g_variant_new_fixed_array (G_VARIANT_TYPE_INT32, - col, + xyt->thetacol, xyt->nrows, - sizeof (col[0]))); + sizeof (xyt->thetacol[0]))); g_variant_builder_close (&nested); - g_free (col); } g_variant_builder_close (&nested); From b5496fd25710225cb966a56ea6fcdc29fc20d983 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Fri, 4 Dec 2020 00:43:12 +0100 Subject: [PATCH 05/22] fp-device: Ensure finger status is set to proper type on property getter Finger status is a flag not an enum. Add tests. --- libfprint/fp-device.c | 2 +- tests/test-fpi-device.c | 13 +++++++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/libfprint/fp-device.c b/libfprint/fp-device.c index 5fe4796d..99d31482 100644 --- a/libfprint/fp-device.c +++ b/libfprint/fp-device.c @@ -193,7 +193,7 @@ fp_device_get_property (GObject *object, break; case PROP_FINGER_STATUS: - g_value_set_enum (value, priv->finger_status); + g_value_set_flags (value, priv->finger_status); break; case PROP_DRIVER: diff --git a/tests/test-fpi-device.c b/tests/test-fpi-device.c index d492bc7a..1854110f 100644 --- a/tests/test-fpi-device.c +++ b/tests/test-fpi-device.c @@ -205,11 +205,16 @@ test_driver_finger_status_inactive (void) { g_autoptr(FpDevice) device = g_object_new (FPI_TYPE_DEVICE_FAKE, NULL); FpiDeviceFake *fake_dev = FPI_DEVICE_FAKE (device); + FpFingerStatusFlags finger_status; g_signal_connect (device, "notify::finger-status", G_CALLBACK (on_device_notify), NULL); g_assert_false (fpi_device_report_finger_status (device, FP_FINGER_STATUS_NONE)); g_assert_cmpuint (fp_device_get_finger_status (device), ==, FP_FINGER_STATUS_NONE); + + g_object_get (fake_dev, "finger-status", &finger_status, NULL); + g_assert_cmpuint (finger_status, ==, FP_FINGER_STATUS_NONE); + g_assert (fake_dev->last_called_function != on_device_notify); g_assert_null (g_steal_pointer (&fake_dev->user_data)); } @@ -220,12 +225,16 @@ test_driver_finger_status_needed (void) g_autoptr(FpDevice) device = g_object_new (FPI_TYPE_DEVICE_FAKE, NULL); g_autoptr(GParamSpec) pspec = NULL; FpiDeviceFake *fake_dev = FPI_DEVICE_FAKE (device); + FpFingerStatusFlags finger_status; g_signal_connect (device, "notify::finger-status", G_CALLBACK (on_device_notify), NULL); g_assert_true (fpi_device_report_finger_status (device, FP_FINGER_STATUS_NEEDED)); g_assert_cmpuint (fp_device_get_finger_status (device), ==, FP_FINGER_STATUS_NEEDED); + g_object_get (fake_dev, "finger-status", &finger_status, NULL); + g_assert_cmpuint (finger_status, ==, FP_FINGER_STATUS_NEEDED); + g_assert (fake_dev->last_called_function == on_device_notify); pspec = g_steal_pointer (&fake_dev->user_data); g_assert_cmpstr (pspec->name, ==, "finger-status"); @@ -242,12 +251,16 @@ test_driver_finger_status_present (void) g_autoptr(FpDevice) device = g_object_new (FPI_TYPE_DEVICE_FAKE, NULL); g_autoptr(GParamSpec) pspec = NULL; FpiDeviceFake *fake_dev = FPI_DEVICE_FAKE (device); + FpFingerStatusFlags finger_status; g_signal_connect (device, "notify::finger-status", G_CALLBACK (on_device_notify), NULL); g_assert_true (fpi_device_report_finger_status (device, FP_FINGER_STATUS_PRESENT)); g_assert_cmpuint (fp_device_get_finger_status (device), ==, FP_FINGER_STATUS_PRESENT); + g_object_get (fake_dev, "finger-status", &finger_status, NULL); + g_assert_cmpuint (finger_status, ==, FP_FINGER_STATUS_PRESENT); + g_assert (fake_dev->last_called_function == on_device_notify); pspec = g_steal_pointer (&fake_dev->user_data); g_assert_cmpstr (pspec->name, ==, "finger-status"); From c1e832e7a79981ec14f15b1beb95f0da43fe1348 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Fri, 4 Dec 2020 00:53:29 +0100 Subject: [PATCH 06/22] fp-device: Return valid finger status value on error Not that the two enums have different value, but indeed the type is wrong. --- libfprint/fp-device.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libfprint/fp-device.c b/libfprint/fp-device.c index 99d31482..a9e8c944 100644 --- a/libfprint/fp-device.c +++ b/libfprint/fp-device.c @@ -545,7 +545,7 @@ fp_device_get_finger_status (FpDevice *device) { FpDevicePrivate *priv = fp_device_get_instance_private (device); - g_return_val_if_fail (FP_IS_DEVICE (device), FP_SCAN_TYPE_SWIPE); + g_return_val_if_fail (FP_IS_DEVICE (device), FP_FINGER_STATUS_NONE); return priv->finger_status; } From 0688288c6d6c513822c8c25895bd08ee2bc3acf1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Mon, 7 Dec 2020 19:01:10 +0100 Subject: [PATCH 07/22] .git-blame-ignore-revs: Ignore formatting commit and add hint how to use it --- .git-blame-ignore-revs | 10 ++++++++++ 1 file changed, 10 insertions(+) create mode 100644 .git-blame-ignore-revs diff --git a/.git-blame-ignore-revs b/.git-blame-ignore-revs new file mode 100644 index 00000000..fdfa6879 --- /dev/null +++ b/.git-blame-ignore-revs @@ -0,0 +1,10 @@ +# The commits that did automated reformatting. You can ignore them +# during git-blame with `--ignore-rev` or `--ignore-revs-file`. +# +# $ git config --add 'blame.ignoreRevsFile' '.git-blame-ignore-revs' +# + +d1fb1e26f3b79e54febc94496c1184763cf2af3d +e4f9935706be4c0e3253afe251c182019ff7ccef +65e602d8c72baa7020efb62d10bf28e621feb05d +4115ae7ced77d392ee11ea55212206d9404356f0 From 91fb8d8cb40e7353c3beda621f1555fb842b543f Mon Sep 17 00:00:00 2001 From: Benjamin Berg Date: Tue, 8 Dec 2020 13:27:50 +0100 Subject: [PATCH 08/22] compat: Add GFlagsClass autopointer It was added to GLib at the same time as GEnumClass. We did not list it though and are now using it in a test. --- libfprint/fpi-compat.h | 1 + 1 file changed, 1 insertion(+) diff --git a/libfprint/fpi-compat.h b/libfprint/fpi-compat.h index 8b87913c..5480eb52 100644 --- a/libfprint/fpi-compat.h +++ b/libfprint/fpi-compat.h @@ -23,6 +23,7 @@ #if !GLIB_CHECK_VERSION (2, 57, 0) G_DEFINE_AUTOPTR_CLEANUP_FUNC (GTypeClass, g_type_class_unref); G_DEFINE_AUTOPTR_CLEANUP_FUNC (GEnumClass, g_type_class_unref); +G_DEFINE_AUTOPTR_CLEANUP_FUNC (GFlagsClass, g_type_class_unref); G_DEFINE_AUTOPTR_CLEANUP_FUNC (GParamSpec, g_param_spec_unref); #else /* Re-define G_SOURCE_FUNC as we are technically not allowed to use it with From 74810a847264f0c05af42c70b2366e386f807590 Mon Sep 17 00:00:00 2001 From: Benjamin Berg Date: Tue, 8 Dec 2020 13:33:30 +0100 Subject: [PATCH 09/22] image: Fix warning about uninitialized variable The variable is only initialized later in the function. This is harmless, as there is no return, but it causes a warning due to the automatic free. --- libfprint/fp-image.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libfprint/fp-image.c b/libfprint/fp-image.c index c2c9742e..51732c1b 100644 --- a/libfprint/fp-image.c +++ b/libfprint/fp-image.c @@ -281,7 +281,7 @@ fp_image_detect_minutiae_thread_func (GTask *task, gint map_w, map_h; gint bw, bh, bd; gint r; - g_autofree LFSPARMS *lfsparms; + g_autofree LFSPARMS *lfsparms = NULL; /* Normalize the image first */ if (data->flags & FPI_IMAGE_H_FLIPPED) From f2ea3e784e483c63b49d5265f3c6f66c87425c42 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Wed, 9 Dec 2020 01:19:33 +0100 Subject: [PATCH 10/22] fp-print: Delete not-defined anymore functions --- doc/libfprint-2-sections.txt | 2 -- libfprint/fp-print.h | 5 ----- 2 files changed, 7 deletions(-) diff --git a/doc/libfprint-2-sections.txt b/doc/libfprint-2-sections.txt index 5325ced0..adb48189 100644 --- a/doc/libfprint-2-sections.txt +++ b/doc/libfprint-2-sections.txt @@ -91,8 +91,6 @@ FP_TYPE_PRINT FpFinger FpPrint fp_print_new -fp_print_new_from_data -fp_print_to_data fp_print_get_driver fp_print_get_device_id fp_print_get_device_stored diff --git a/libfprint/fp-print.h b/libfprint/fp-print.h index 39cf87c6..ac6820d7 100644 --- a/libfprint/fp-print.h +++ b/libfprint/fp-print.h @@ -80,11 +80,6 @@ typedef enum { FpPrint *fp_print_new (FpDevice *device); -FpPrint *fp_print_new_from_data (guchar *data, - gsize length); -gboolean fp_print_to_data (guchar **data, - gsize length); - const gchar *fp_print_get_driver (FpPrint *print); const gchar *fp_print_get_device_id (FpPrint *print); FpImage *fp_print_get_image (FpPrint *print); From 8112da03585d26cacfcffd960b5763e0285c14ef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Wed, 9 Dec 2020 01:20:10 +0100 Subject: [PATCH 11/22] umockdev-tests: Still raise an error when storing the exception output Otherwise we won't ever fail --- tests/umockdev-test.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/umockdev-test.py b/tests/umockdev-test.py index dc759952..a25f995a 100755 --- a/tests/umockdev-test.py +++ b/tests/umockdev-test.py @@ -87,10 +87,11 @@ try: if os.path.exists(os.path.join(ddir, "custom.ioctl")): custom() -except: +except Exception as e: # Store created output files for inspection (in the build directory) outdir = os.path.join('errors', os.path.basename(ddir)) shutil.copytree(tmpdir, outdir) + raise e finally: shutil.rmtree(tmpdir) From 6ca8441df9abc38af4e42f990ac14d83836f9445 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Wed, 9 Dec 2020 01:21:49 +0100 Subject: [PATCH 12/22] umockdev-tests: Don't fail when trying to save other errors --- tests/umockdev-test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) mode change 100755 => 100644 tests/umockdev-test.py diff --git a/tests/umockdev-test.py b/tests/umockdev-test.py old mode 100755 new mode 100644 index a25f995a..26dab613 --- a/tests/umockdev-test.py +++ b/tests/umockdev-test.py @@ -90,7 +90,7 @@ try: except Exception as e: # Store created output files for inspection (in the build directory) outdir = os.path.join('errors', os.path.basename(ddir)) - shutil.copytree(tmpdir, outdir) + shutil.copytree(tmpdir, outdir, dirs_exist_ok=True) raise e finally: From fb23f8690fb7145e3be0eb1ea8ebc8e3565344c3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Wed, 9 Dec 2020 02:01:23 +0100 Subject: [PATCH 13/22] fp-print: Return NULL on error not really different from FALSE, but still.. --- libfprint/fp-print.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libfprint/fp-print.c b/libfprint/fp-print.c index 70775c93..a4e8350c 100644 --- a/libfprint/fp-print.c +++ b/libfprint/fp-print.c @@ -878,5 +878,5 @@ invalid_format: *error = g_error_new_literal (G_IO_ERROR, G_IO_ERROR_INVALID_DATA, "Data could not be parsed"); - return FALSE; + return NULL; } From 0d9d7dcb4601a60b464c2c257a6febc376503067 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Wed, 9 Dec 2020 02:02:45 +0100 Subject: [PATCH 14/22] fp-print: Don't deference the passed error, use g_set_error instead It still may be NULL, but we don't protect from that. --- libfprint/fp-print.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/libfprint/fp-print.c b/libfprint/fp-print.c index a4e8350c..522e942f 100644 --- a/libfprint/fp-print.c +++ b/libfprint/fp-print.c @@ -875,8 +875,7 @@ fp_print_deserialize (const guchar *data, return g_steal_pointer (&result); invalid_format: - *error = g_error_new_literal (G_IO_ERROR, - G_IO_ERROR_INVALID_DATA, - "Data could not be parsed"); + g_set_error (error, G_IO_ERROR, G_IO_ERROR_INVALID_DATA, + "Data could not be parsed"); return NULL; } From 0ff7a0767115611bf7fa04c3e8cca786d1398276 Mon Sep 17 00:00:00 2001 From: fengqiangguo Date: Wed, 9 Dec 2020 17:54:09 +0800 Subject: [PATCH 15/22] goodixmoc: fix package crc error fix package length type convert error --- libfprint/drivers/goodixmoc/goodix_proto.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libfprint/drivers/goodixmoc/goodix_proto.c b/libfprint/drivers/goodixmoc/goodix_proto.c index 3174170b..ece8123d 100644 --- a/libfprint/drivers/goodixmoc/goodix_proto.c +++ b/libfprint/drivers/goodixmoc/goodix_proto.c @@ -229,7 +229,7 @@ gx_proto_parse_header ( memcpy (pheader, buffer, sizeof (pack_header)); - pheader->len = GUINT16_FROM_LE (*(buffer + 4)); + pheader->len = GUINT16_FROM_LE ( *(uint16_t *) (buffer + 4)); pheader->len -= PACKAGE_CRC_SIZE; return 0; From 499de3e442dc38f1f179170e7b3acc21303cdc51 Mon Sep 17 00:00:00 2001 From: Benjamin Berg Date: Wed, 9 Dec 2020 10:46:36 +0100 Subject: [PATCH 16/22] print: Return sunk reference from deserialize function This function was always documented to return a sunk reference, but it did not do so. This change is technically backward incompatible. However, it only has an effect if anything is doing a g_object_ref_sink. Which may happen inside libfprint itself. With the change, most API users (including fprintd) are fixed to do refcounting correctly. Any API user which worked around this will have a memory leak now. That is not ideal, but it is not really that bad overall. And returning a floating reference for FpPrint creation was a bad idea in the first place. And it really only makes sense for fp_print_new as the only (public) use case is to create the template for enrollment. --- libfprint/fp-print.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/libfprint/fp-print.c b/libfprint/fp-print.c index 522e942f..9c2c5374 100644 --- a/libfprint/fp-print.c +++ b/libfprint/fp-print.c @@ -808,6 +808,7 @@ fp_print_deserialize (const guchar *data, "device-id", device_id, "device-stored", device_stored, NULL); + g_object_ref_sink (result); fpi_print_set_type (result, FPI_PRINT_NBIS); for (i = 0; i < g_variant_n_children (prints); i++) { @@ -857,6 +858,7 @@ fp_print_deserialize (const guchar *data, "device-stored", device_stored, "fpi-data", fp_data, NULL); + g_object_ref_sink (result); } else { From faade91c3937204a16e6fed43be91ee6a70820b5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Wed, 9 Dec 2020 11:13:04 +0100 Subject: [PATCH 17/22] test-fpi-device: Add function to create fake FpPrint's and galleries --- tests/test-fpi-device.c | 156 ++++++++++++++++++++++++---------------- 1 file changed, 96 insertions(+), 60 deletions(-) diff --git a/tests/test-fpi-device.c b/tests/test-fpi-device.c index 1854110f..1018ae0e 100644 --- a/tests/test-fpi-device.c +++ b/tests/test-fpi-device.c @@ -89,6 +89,29 @@ auto_reset_device_class_cleanup (FpAutoResetClass *dev_class) } G_DEFINE_AUTOPTR_CLEANUP_FUNC (FpAutoResetClass, auto_reset_device_class_cleanup) + +static void +assert_equal_galleries (GPtrArray *g1, + GPtrArray *g2) +{ + unsigned i; + + g_assert ((g1 && g2) || (!g1 || !g1)); + + if (g1 == g2) + return; + + g_assert_cmpuint (g1->len, ==, g2->len); + + for (i = 0; i < g1->len; i++) + { + FpPrint *print = g_ptr_array_index (g1, i); + + g_assert_true (g_ptr_array_find_with_equal_func (g2, print, (GEqualFunc) + fp_print_equal, NULL)); + } +} + static void on_device_notify (FpDevice *device, GParamSpec *spec, gpointer user_data) { @@ -98,6 +121,43 @@ on_device_notify (FpDevice *device, GParamSpec *spec, gpointer user_data) fake_dev->user_data = g_param_spec_ref (spec); } +static FpPrint * +make_fake_print (FpDevice *device, + GVariant *print_data) +{ + FpPrint *enrolled_print = fp_print_new (device); + + fpi_print_set_type (enrolled_print, FPI_PRINT_RAW); + + if (!print_data) + print_data = g_variant_new_string ("Test print private data"); + g_object_set (G_OBJECT (enrolled_print), "fpi-data", print_data, NULL); + + return enrolled_print; +} + +static FpPrint * +make_fake_print_reffed (FpDevice *device, + GVariant *print_data) +{ + return g_object_ref_sink (make_fake_print (device, print_data)); +} + +static GPtrArray * +make_fake_prints_gallery (FpDevice *device, + size_t size) +{ + GPtrArray *array; + size_t i; + + array = g_ptr_array_new_full (size, g_object_unref); + + for (i = 0; i < size; i++) + g_ptr_array_add (array, make_fake_print_reffed (device, g_variant_new_uint64 (i))); + + return array; +} + /* Tests */ static void @@ -683,7 +743,7 @@ test_driver_enroll_error_no_print (void) "*Driver passed an error but also provided a print, returning error*"); fake_dev->ret_error = fpi_device_error_new (FP_DEVICE_ERROR_GENERAL); - fake_dev->ret_print = fp_print_new (device); + fake_dev->ret_print = make_fake_print_reffed (device, NULL); g_object_add_weak_pointer (G_OBJECT (fake_dev->ret_print), (gpointer) (&fake_dev->ret_print)); out_print = @@ -774,7 +834,8 @@ test_driver_enroll_progress_vfunc (FpDevice *device) expected_data->completed_stages = g_random_int_range (fp_device_get_nr_enroll_stages (device), G_MAXINT32); - expected_data->print = fp_print_new (device); + expected_data->print = make_fake_print_reffed (device, + g_variant_new_int32 (expected_data->completed_stages)); expected_data->error = NULL; error = fpi_device_error_new (FP_DEVICE_ERROR_GENERAL); @@ -885,7 +946,7 @@ test_driver_verify (void) { g_autoptr(GError) error = NULL; g_autoptr(FpAutoCloseDevice) device = auto_close_fake_device_new (); - g_autoptr(FpPrint) enrolled_print = g_object_ref_sink (fp_print_new (device)); + g_autoptr(FpPrint) enrolled_print = make_fake_print_reffed (device, NULL); g_autoptr(FpPrint) out_print = NULL; g_autoptr(MatchCbData) match_data = g_new0 (MatchCbData, 1); FpDeviceClass *dev_class = FP_DEVICE_GET_CLASS (device); @@ -915,13 +976,14 @@ test_driver_verify_fail (void) { g_autoptr(GError) error = NULL; g_autoptr(FpAutoCloseDevice) device = auto_close_fake_device_new (); - g_autoptr(FpPrint) enrolled_print = g_object_ref_sink (fp_print_new (device)); + g_autoptr(FpPrint) enrolled_print = NULL; g_autoptr(FpPrint) out_print = NULL; g_autoptr(MatchCbData) match_data = g_new0 (MatchCbData, 1); FpDeviceClass *dev_class = FP_DEVICE_GET_CLASS (device); FpiDeviceFake *fake_dev = FPI_DEVICE_FAKE (device); gboolean match; + enrolled_print = make_fake_print_reffed (device, g_variant_new_uint64 (3)); fake_dev->ret_result = FPI_MATCH_FAIL; g_assert_true (fp_device_verify_sync (device, enrolled_print, NULL, test_driver_match_cb, match_data, @@ -944,7 +1006,7 @@ test_driver_verify_retry (void) { g_autoptr(GError) error = NULL; g_autoptr(FpAutoCloseDevice) device = auto_close_fake_device_new (); - g_autoptr(FpPrint) enrolled_print = g_object_ref_sink (fp_print_new (device)); + g_autoptr(FpPrint) enrolled_print = make_fake_print_reffed (device, NULL); g_autoptr(FpPrint) out_print = NULL; g_autoptr(MatchCbData) match_data = g_new0 (MatchCbData, 1); FpDeviceClass *dev_class = FP_DEVICE_GET_CLASS (device); @@ -972,7 +1034,7 @@ test_driver_verify_error (void) { g_autoptr(GError) error = NULL; g_autoptr(FpAutoCloseDevice) device = auto_close_fake_device_new (); - g_autoptr(FpPrint) enrolled_print = g_object_ref_sink (fp_print_new (device)); + g_autoptr(FpPrint) enrolled_print = make_fake_print_reffed (device, NULL); g_autoptr(FpPrint) out_print = NULL; g_autoptr(MatchCbData) match_data = g_new0 (MatchCbData, 1); FpDeviceClass *dev_class = FP_DEVICE_GET_CLASS (device); @@ -1011,7 +1073,7 @@ test_driver_verify_not_reported (void) dev_class->verify = fake_device_verify_immediate_complete; device = g_object_new (FPI_TYPE_DEVICE_FAKE, NULL); - enrolled_print = g_object_ref_sink (fp_print_new (device)); + enrolled_print = make_fake_print_reffed (device, NULL); g_assert_true (fp_device_open_sync (device, NULL, NULL)); @@ -1054,7 +1116,7 @@ test_driver_verify_report_no_callback (void) dev_class->verify = fake_device_verify_complete_error; device = g_object_new (FPI_TYPE_DEVICE_FAKE, NULL); fake_dev = FPI_DEVICE_FAKE (device); - enrolled_print = g_object_ref_sink (fp_print_new (device)); + enrolled_print = make_fake_print_reffed (device, NULL); g_assert_true (fp_device_open_sync (device, NULL, NULL)); @@ -1094,7 +1156,7 @@ test_driver_verify_complete_retry (void) dev_class->verify = fake_device_verify_complete_error; device = g_object_new (FPI_TYPE_DEVICE_FAKE, NULL); fake_dev = FPI_DEVICE_FAKE (device); - enrolled_print = g_object_ref_sink (fp_print_new (device)); + enrolled_print = make_fake_print_reffed (device, NULL); g_assert_true (fp_device_open_sync (device, NULL, NULL)); @@ -1187,7 +1249,7 @@ test_driver_verify_complete_retry (void) test_driver_match_data_clear (match_data); fake_dev->ret_result = FPI_MATCH_ERROR; fake_dev->ret_error = fpi_device_retry_new (FP_DEVICE_RETRY_TOO_SHORT); - fake_dev->ret_print = fp_print_new (device); + fake_dev->ret_print = make_fake_print (device, NULL); g_object_add_weak_pointer (G_OBJECT (fake_dev->ret_print), (gpointer) (&fake_dev->ret_print)); @@ -1241,22 +1303,18 @@ test_driver_identify (void) g_autoptr(FpPrint) print = NULL; g_autoptr(FpPrint) matched_print = NULL; g_autoptr(FpAutoCloseDevice) device = auto_close_fake_device_new (); - g_autoptr(GPtrArray) prints = g_ptr_array_new_with_free_func (g_object_unref); + g_autoptr(GPtrArray) prints = make_fake_prints_gallery (device, 500); g_autoptr(MatchCbData) match_data = g_new0 (MatchCbData, 1); FpDeviceClass *dev_class = FP_DEVICE_GET_CLASS (device); FpiDeviceFake *fake_dev = FPI_DEVICE_FAKE (device); FpPrint *expected_matched; - unsigned int i; - - for (i = 0; i < 500; ++i) - g_ptr_array_add (prints, g_object_ref_sink (fp_print_new (device))); expected_matched = g_ptr_array_index (prints, g_random_int_range (0, 499)); fp_print_set_description (expected_matched, "fake-verified"); g_assert_true (fp_device_supports_identify (device)); - fake_dev->ret_print = fp_print_new (device); + fake_dev->ret_print = make_fake_print (device, NULL); g_assert_true (fp_device_identify_sync (device, prints, NULL, test_driver_match_cb, match_data, &matched_print, &print, &error)); @@ -1281,18 +1339,14 @@ test_driver_identify_fail (void) g_autoptr(FpPrint) print = NULL; g_autoptr(FpPrint) matched_print = NULL; g_autoptr(FpAutoCloseDevice) device = auto_close_fake_device_new (); - g_autoptr(GPtrArray) prints = g_ptr_array_new_with_free_func (g_object_unref); + g_autoptr(GPtrArray) prints = make_fake_prints_gallery (device, 500); g_autoptr(MatchCbData) match_data = g_new0 (MatchCbData, 1); FpDeviceClass *dev_class = FP_DEVICE_GET_CLASS (device); FpiDeviceFake *fake_dev = FPI_DEVICE_FAKE (device); - unsigned int i; - - for (i = 0; i < 500; ++i) - g_ptr_array_add (prints, g_object_ref_sink (fp_print_new (device))); g_assert_true (fp_device_supports_identify (device)); - fake_dev->ret_print = fp_print_new (device); + fake_dev->ret_print = make_fake_print (device, NULL); g_assert_true (fp_device_identify_sync (device, prints, NULL, test_driver_match_cb, match_data, &matched_print, &print, &error)); @@ -1317,15 +1371,11 @@ test_driver_identify_retry (void) g_autoptr(FpPrint) print = NULL; g_autoptr(FpPrint) matched_print = NULL; g_autoptr(FpAutoCloseDevice) device = auto_close_fake_device_new (); - g_autoptr(GPtrArray) prints = g_ptr_array_new_with_free_func (g_object_unref); + g_autoptr(GPtrArray) prints = make_fake_prints_gallery (device, 500); g_autoptr(MatchCbData) match_data = g_new0 (MatchCbData, 1); FpDeviceClass *dev_class = FP_DEVICE_GET_CLASS (device); FpiDeviceFake *fake_dev = FPI_DEVICE_FAKE (device); FpPrint *expected_matched; - unsigned int i; - - for (i = 0; i < 500; ++i) - g_ptr_array_add (prints, g_object_ref_sink (fp_print_new (device))); expected_matched = g_ptr_array_index (prints, g_random_int_range (0, 499)); fp_print_set_description (expected_matched, "fake-verified"); @@ -1355,15 +1405,11 @@ test_driver_identify_error (void) g_autoptr(FpPrint) print = NULL; g_autoptr(FpPrint) matched_print = NULL; g_autoptr(FpAutoCloseDevice) device = auto_close_fake_device_new (); - g_autoptr(GPtrArray) prints = g_ptr_array_new_with_free_func (g_object_unref); + g_autoptr(GPtrArray) prints = make_fake_prints_gallery (device, 500); g_autoptr(MatchCbData) match_data = g_new0 (MatchCbData, 1); FpDeviceClass *dev_class = FP_DEVICE_GET_CLASS (device); FpiDeviceFake *fake_dev = FPI_DEVICE_FAKE (device); FpPrint *expected_matched; - unsigned int i; - - for (i = 0; i < 500; ++i) - g_ptr_array_add (prints, g_object_ref_sink (fp_print_new (device))); expected_matched = g_ptr_array_index (prints, g_random_int_range (0, 499)); fp_print_set_description (expected_matched, "fake-verified"); @@ -1397,15 +1443,12 @@ test_driver_identify_not_reported (void) { g_autoptr(FpAutoResetClass) dev_class = auto_reset_device_class (); g_autoptr(FpAutoCloseDevice) device = NULL; - g_autoptr(GPtrArray) prints = g_ptr_array_new_with_free_func (g_object_unref); + g_autoptr(GPtrArray) prints = NULL; g_autoptr(GError) error = NULL; - unsigned int i; dev_class->identify = fake_device_identify_immediate_complete; device = g_object_new (FPI_TYPE_DEVICE_FAKE, NULL); - - for (i = 0; i < 500; ++i) - g_ptr_array_add (prints, g_object_ref_sink (fp_print_new (device))); + prints = make_fake_prints_gallery (device, 500); g_assert_true (fp_device_open_sync (device, NULL, NULL)); @@ -1437,21 +1480,18 @@ static void test_driver_identify_complete_retry (void) { g_autoptr(FpAutoResetClass) dev_class = auto_reset_device_class (); - g_autoptr(GPtrArray) prints = g_ptr_array_new_with_free_func (g_object_unref); + g_autoptr(GPtrArray) prints = NULL; g_autoptr(MatchCbData) match_data = g_new0 (MatchCbData, 1); g_autoptr(FpAutoCloseDevice) device = NULL; g_autoptr(FpPrint) print = NULL; g_autoptr(FpPrint) match = NULL; g_autoptr(GError) error = NULL; FpiDeviceFake *fake_dev; - int i; dev_class->identify = fake_device_identify_complete_error; device = g_object_new (FPI_TYPE_DEVICE_FAKE, NULL); fake_dev = FPI_DEVICE_FAKE (device); - - for (i = 0; i < 500; ++i) - g_ptr_array_add (prints, g_object_ref_sink (fp_print_new (device))); + prints = make_fake_prints_gallery (device, 500); g_assert_true (fp_device_open_sync (device, NULL, NULL)); @@ -1481,7 +1521,7 @@ test_driver_identify_complete_retry (void) "*Driver reported a match to a print that was not in the gallery*"); test_driver_match_data_clear (match_data); - fake_dev->ret_match = fp_print_new (device); + fake_dev->ret_match = make_fake_print_reffed (device, NULL); g_object_add_weak_pointer (G_OBJECT (fake_dev->ret_match), (gpointer) (&fake_dev->ret_match)); g_assert_true (fp_device_identify_sync (device, prints, NULL, @@ -1505,7 +1545,7 @@ test_driver_identify_complete_retry (void) test_driver_match_data_clear (match_data); fake_dev->ret_error = fpi_device_retry_new (FP_DEVICE_RETRY_REMOVE_FINGER); fake_dev->ret_match = prints->pdata[0]; - fake_dev->ret_print = fp_print_new (device); + fake_dev->ret_print = make_fake_print (device, NULL); g_object_add_weak_pointer (G_OBJECT (fake_dev->ret_print), (gpointer) (&fake_dev->ret_print)); g_assert_false (fp_device_identify_sync (device, prints, NULL, @@ -1528,7 +1568,7 @@ test_driver_identify_report_no_callback (void) { g_autoptr(FpAutoResetClass) dev_class = auto_reset_device_class (); g_autoptr(MatchCbData) match_data = g_new0 (MatchCbData, 1); - g_autoptr(GPtrArray) prints = g_ptr_array_new_with_free_func (g_object_unref); + g_autoptr(GPtrArray) prints = NULL; g_autoptr(FpAutoCloseDevice) device = NULL; G_GNUC_UNUSED g_autoptr(FpPrint) enrolled_print = NULL; g_autoptr(FpPrint) print = NULL; @@ -1539,7 +1579,8 @@ test_driver_identify_report_no_callback (void) dev_class->identify = fake_device_identify_complete_error; device = g_object_new (FPI_TYPE_DEVICE_FAKE, NULL); fake_dev = FPI_DEVICE_FAKE (device); - enrolled_print = g_object_ref_sink (fp_print_new (device)); + prints = make_fake_prints_gallery (device, 0); + enrolled_print = make_fake_print_reffed (device, NULL); g_assert_true (fp_device_open_sync (device, NULL, NULL)); @@ -1666,15 +1707,10 @@ test_driver_list (void) { g_autoptr(GError) error = NULL; g_autoptr(FpAutoCloseDevice) device = auto_close_fake_device_new (); + g_autoptr(GPtrArray) prints = make_fake_prints_gallery (device, 500); FpDeviceClass *dev_class = FP_DEVICE_GET_CLASS (device); FpiDeviceFake *fake_dev = FPI_DEVICE_FAKE (device); - g_autoptr(GPtrArray) prints = g_ptr_array_new_with_free_func (g_object_unref); - unsigned int i; - - for (i = 0; i < 500; ++i) - g_ptr_array_add (prints, fp_print_new (device)); - fake_dev->ret_list = g_steal_pointer (&prints); prints = fp_device_list_prints_sync (device, NULL, &error); @@ -1727,7 +1763,7 @@ test_driver_delete (void) { g_autoptr(GError) error = NULL; g_autoptr(FpAutoCloseDevice) device = auto_close_fake_device_new (); - g_autoptr(FpPrint) enrolled_print = fp_print_new (device); + g_autoptr(FpPrint) enrolled_print = make_fake_print_reffed (device, NULL); FpDeviceClass *dev_class = FP_DEVICE_GET_CLASS (device); FpiDeviceFake *fake_dev = FPI_DEVICE_FAKE (device); gboolean ret; @@ -1744,7 +1780,7 @@ test_driver_delete_error (void) { g_autoptr(GError) error = NULL; g_autoptr(FpAutoCloseDevice) device = auto_close_fake_device_new (); - g_autoptr(FpPrint) enrolled_print = fp_print_new (device); + g_autoptr(FpPrint) enrolled_print = make_fake_print_reffed (device, NULL); FpDeviceClass *dev_class = FP_DEVICE_GET_CLASS (device); FpiDeviceFake *fake_dev = FPI_DEVICE_FAKE (device); gboolean ret; @@ -1812,7 +1848,7 @@ test_driver_cancel (void) device = auto_close_fake_device_new (); fake_dev = FPI_DEVICE_FAKE (device); cancellable = g_cancellable_new (); - enrolled_print = fp_print_new (device); + enrolled_print = make_fake_print_reffed (device, NULL); fp_device_delete_print (device, enrolled_print, cancellable, on_driver_cancel_delete, &completed); @@ -1830,7 +1866,7 @@ test_driver_cancel_fail (void) g_autoptr(GError) error = NULL; g_autoptr(FpAutoCloseDevice) device = auto_close_fake_device_new (); g_autoptr(GCancellable) cancellable = g_cancellable_new (); - g_autoptr(FpPrint) enrolled_print = fp_print_new (device); + g_autoptr(FpPrint) enrolled_print = make_fake_print_reffed (device, NULL); FpDeviceClass *dev_class = FP_DEVICE_GET_CLASS (device); FpiDeviceFake *fake_dev = FPI_DEVICE_FAKE (device); @@ -2064,8 +2100,8 @@ static void test_driver_action_error_all (void) { g_autoptr(FpAutoCloseDevice) device = g_object_new (FPI_TYPE_DEVICE_FAKE, NULL); - g_autoptr(FpPrint) enrolled_print = g_object_ref_sink (fp_print_new (device)); - g_autoptr(GPtrArray) prints = g_ptr_array_new_with_free_func (g_object_unref); + g_autoptr(FpPrint) enrolled_print = make_fake_print_reffed (device, NULL); + g_autoptr(GPtrArray) prints = make_fake_prints_gallery (device, 0); g_autoptr(GError) error = NULL; FpDeviceClass *dev_class = FP_DEVICE_GET_CLASS (device); FpiDeviceFake *fake_dev; @@ -2136,8 +2172,8 @@ static void test_driver_action_error_fallback_all (void) { g_autoptr(FpAutoCloseDevice) device = g_object_new (FPI_TYPE_DEVICE_FAKE, NULL); - g_autoptr(FpPrint) enrolled_print = g_object_ref_sink (fp_print_new (device)); - g_autoptr(GPtrArray) prints = g_ptr_array_new_with_free_func (g_object_unref); + g_autoptr(FpPrint) enrolled_print = make_fake_print_reffed (device, NULL); + g_autoptr(GPtrArray) prints = make_fake_prints_gallery (device, 0); g_autoptr(GError) error = NULL; FpDeviceClass *dev_class = FP_DEVICE_GET_CLASS (device); FpiDeviceFake *fake_dev; From 28ba6a0de93c8e71fadac1c13e9eb11fae424fd9 Mon Sep 17 00:00:00 2001 From: Benjamin Berg Date: Wed, 9 Dec 2020 11:43:17 +0100 Subject: [PATCH 18/22] test-fpi-device: Do deep comparison of gallery The gallery needs to be copied, as such we must do a deep comparison instead of comparing the pointers. We also can't do the comparison afterwards, as the gallery is owned by the operation and that operation is finished already. --- tests/test-fpi-device.c | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/tests/test-fpi-device.c b/tests/test-fpi-device.c index 1018ae0e..57e5cf91 100644 --- a/tests/test-fpi-device.c +++ b/tests/test-fpi-device.c @@ -893,10 +893,11 @@ test_driver_enroll_progress (void) typedef struct { - gboolean called; - FpPrint *match; - FpPrint *print; - GError *error; + gboolean called; + FpPrint *match; + FpPrint *print; + GPtrArray *gallery; + GError *error; } MatchCbData; static void @@ -939,6 +940,14 @@ test_driver_match_cb (FpDevice *device, if (match) g_assert_no_error (error); + + /* Compar gallery if this is an identify operation */ + if (data->gallery) + { + FpiDeviceFake *fake_dev = FPI_DEVICE_FAKE (device); + g_assert_false (fake_dev->action_data == data->gallery); + assert_equal_galleries (fake_dev->action_data, data->gallery); + } } static void @@ -1314,6 +1323,8 @@ test_driver_identify (void) g_assert_true (fp_device_supports_identify (device)); + match_data->gallery = prints; + fake_dev->ret_print = make_fake_print (device, NULL); g_assert_true (fp_device_identify_sync (device, prints, NULL, test_driver_match_cb, match_data, @@ -1325,7 +1336,6 @@ test_driver_identify (void) g_assert_true (match_data->print == print); g_assert (fake_dev->last_called_function == dev_class->identify); - g_assert (fake_dev->action_data == prints); g_assert_no_error (error); g_assert (print != NULL && print == fake_dev->ret_print); From 91ee03eb7a36a50cdd65d21cabf0d77a80870d58 Mon Sep 17 00:00:00 2001 From: Benjamin Berg Date: Wed, 9 Dec 2020 10:55:52 +0100 Subject: [PATCH 19/22] device: Fix memory management of gallery passed to identify We cannot make any assumptions about the passed GPtrArray. As such, we must copy the content and grab our own reference for each of the prints. --- libfprint/fp-device.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/libfprint/fp-device.c b/libfprint/fp-device.c index a9e8c944..6db665fa 100644 --- a/libfprint/fp-device.c +++ b/libfprint/fp-device.c @@ -1018,6 +1018,7 @@ fp_device_identify (FpDevice *device, g_autoptr(GTask) task = NULL; FpDevicePrivate *priv = fp_device_get_instance_private (device); FpMatchData *data; + int i; task = g_task_new (device, cancellable, callback, user_data); if (g_task_return_error_if_cancelled (task)) @@ -1042,7 +1043,13 @@ fp_device_identify (FpDevice *device, maybe_cancel_on_cancelled (device, cancellable); data = g_new0 (FpMatchData, 1); - data->gallery = g_ptr_array_ref (prints); + /* We cannot store the gallery directly, because the ptr array may not own + * a reference to each print. Also, the caller could in principle modify the + * GPtrArray afterwards. + */ + data->gallery = g_ptr_array_new_full (prints->len, g_object_unref); + for (i = 0; i < prints->len; i++) + g_ptr_array_add (data->gallery, g_object_ref (g_ptr_array_index (prints, i))); data->match_cb = match_cb; data->match_data = match_data; data->match_destroy = match_destroy; From 989d498eb9a15470bf76dbc8e7134e999d3018e5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Mon, 7 Dec 2020 21:22:10 +0100 Subject: [PATCH 20/22] goodix: Don't leak the templates array during verify When verifying we initialize a temporary templates array but we never release it. --- libfprint/drivers/goodixmoc/goodix.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/libfprint/drivers/goodixmoc/goodix.c b/libfprint/drivers/goodixmoc/goodix.c index 6a51cb88..bb0368b9 100644 --- a/libfprint/drivers/goodixmoc/goodix.c +++ b/libfprint/drivers/goodixmoc/goodix.c @@ -350,9 +350,9 @@ fp_verify_cb (FpiDeviceGoodixMoc *self, gxfp_cmd_response_t *resp, GError *error) { + g_autoptr(GPtrArray) templates = NULL; FpDevice *device = FP_DEVICE (self); FpPrint *print = NULL; - GPtrArray *templates = NULL; gint cnt = 0; gboolean find = false; @@ -365,15 +365,14 @@ fp_verify_cb (FpiDeviceGoodixMoc *self, { if (fpi_device_get_current_action (device) == FPI_DEVICE_ACTION_VERIFY) { - - templates = g_ptr_array_new_with_free_func (g_object_unref); + templates = g_ptr_array_sized_new (1); fpi_device_get_verify_data (device, &print); - g_ptr_array_add (templates, g_object_ref_sink (print)); - + g_ptr_array_add (templates, print); } else { fpi_device_get_identify_data (device, &templates); + g_ptr_array_ref (templates); } for (cnt = 0; cnt < templates->len; cnt++) { From c02771d16bbc66db7739a831b8e5881382a5356d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Wed, 9 Dec 2020 01:36:24 +0100 Subject: [PATCH 21/22] goodixmoc: Add async identification test using on-owned deseralized prints This simulates what fprintd does --- tests/goodixmoc/custom.py | 27 +++++++++++++++++++++++---- 1 file changed, 23 insertions(+), 4 deletions(-) diff --git a/tests/goodixmoc/custom.py b/tests/goodixmoc/custom.py index 6cc3b702..57faf6c5 100755 --- a/tests/goodixmoc/custom.py +++ b/tests/goodixmoc/custom.py @@ -22,6 +22,13 @@ template = FPrint.Print.new(d) def enroll_progress(*args): print('enroll progress: ' + str(args)) +def identify_done(dev, res): + global identified + identified = True + identify_match, identify_print = dev.identify_finish(res) + print('indentification_done: ', identify_match, identify_print) + assert identify_match.equal(identify_print) + # List, enroll, list, verify, identify, delete print("enrolling") p = d.enroll_sync(template, None, enroll_progress, None) @@ -35,15 +42,27 @@ assert stored[0].equal(p) print("verifying") verify_res, verify_print = d.verify_sync(p) print("verify done") +del p assert verify_res == True -print("identifying") -identify_match, identify_print = d.identify_sync(stored) -print("identify done") -assert identify_match.equal(identify_print) + +identified = False +deserialized_prints = [] +for p in stored: + deserialized_prints.append(FPrint.Print.deserialize(p.serialize())) + assert deserialized_prints[-1].equal(p) +del stored + +print('async identifying') +d.identify(deserialized_prints, callback=identify_done) +del deserialized_prints + +while not identified: + ctx.iteration(True) print("deleting") d.delete_print_sync(p) print("delete done") + d.close_sync() del d From c96958582f1f752702c4aaf2a51c4905213a9bd3 Mon Sep 17 00:00:00 2001 From: Benjamin Berg Date: Wed, 9 Dec 2020 13:10:06 +0100 Subject: [PATCH 22/22] Release 1.90.6 --- NEWS | 18 ++++++++++++++++++ meson.build | 2 +- 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/NEWS b/NEWS index e33b3f34..335986ed 100644 --- a/NEWS +++ b/NEWS @@ -1,6 +1,24 @@ This file lists notable changes in each release. For the full history of all changes, see ChangeLog. +2020-12-01: v1.90.6 release + +This release is primarily a bugfix release for some older issues. + +The major change is that fp_print_deserialize will now correctly return a +sunken reference rather than a floating one. Most API users will have +assumed this was true, and issues could happen at a later point. +If any API user worked around this libfprint bug, they will now leak the +returned print. + +Highlights: + * Object reference management fixes for FpPrint and identify + * Fixed issues that caused problem on non-x86 machines (#236) + * Fix building with older GLib versions + * synaptics: Support PID 00e7 + * goodix: Fix issue with long USB packages + + 2020-12-01: v1.90.5 release The 1.90.4 release caused a major regression, as it included a USB hub in diff --git a/meson.build b/meson.build index 502fe042..a2b849b1 100644 --- a/meson.build +++ b/meson.build @@ -1,5 +1,5 @@ project('libfprint', [ 'c', 'cpp' ], - version: '1.90.5', + version: '1.90.6', license: 'LGPLv2.1+', default_options: [ 'buildtype=debugoptimized',