Bug 57829 - fixes to async state machines
Summary: fixes to async state machines
Status: RESOLVED FIXED
Alias: None
Product: libfprint
Classification: Unclassified
Component: libfprint (show other bugs)
Version: unspecified
Hardware: All All
: medium normal
Assignee: libfprint-bugs
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-12-03 08:02 UTC by Timo Teräs
Modified: 2012-12-14 12:18 UTC (History)
1 user (show)

See Also:
i915 platform:
i915 features:


Attachments
uru4000: fix cancelling of imaging from error callback (1.38 KB, patch)
2012-12-03 08:02 UTC, Timo Teräs
Details | Splinter Review
imgdev: fix cancelling of enrollment from stage_completed callback (1.15 KB, patch)
2012-12-03 08:03 UTC, Timo Teräs
Details | Splinter Review
Update AUTHORS (679 bytes, patch)
2012-12-03 08:04 UTC, Timo Teräs
Details | Splinter Review
imgdev: fix cancelling of enrollment from stage_completed callback [v2] (1.31 KB, patch)
2012-12-03 09:54 UTC, Timo Teräs
Details | Splinter Review
imgdev: fix cancelling of enrollment from stage_completed callback [v3] (1.31 KB, patch)
2012-12-03 11:08 UTC, Timo Teräs
Details | Splinter Review

Description Timo Teräs 2012-12-03 08:02:57 UTC
Created attachment 70953 [details] [review]
uru4000: fix cancelling of imaging from error callback

Fix cancelling of enrollment from non-completing stage callbacks for imaging devices.
Fix uru4000 imaging cancelling from error callback.
Update AUTHORS.
Comment 1 Timo Teräs 2012-12-03 08:03:30 UTC
Created attachment 70954 [details] [review]
imgdev: fix cancelling of enrollment from stage_completed callback
Comment 2 Timo Teräs 2012-12-03 08:04:09 UTC
Created attachment 70955 [details] [review]
Update AUTHORS
Comment 3 Vasily Khoruzhick 2012-12-03 09:06:41 UTC
Comment on attachment 70953 [details] [review]
uru4000: fix cancelling of imaging from error callback

Review of attachment 70953 [details] [review]:
-----------------------------------------------------------------

Looks OK
Comment 4 Vasily Khoruzhick 2012-12-03 09:32:31 UTC
Comment on attachment 70955 [details] [review]
Update AUTHORS

Review of attachment 70955 [details] [review]:
-----------------------------------------------------------------

Looks OK
Comment 5 Vasily Khoruzhick 2012-12-03 09:33:50 UTC
Comment on attachment 70954 [details] [review]
imgdev: fix cancelling of enrollment from stage_completed callback

Review of attachment 70954 [details] [review]:
-----------------------------------------------------------------

--

::: libfprint/imgdev.c
@@ +146,5 @@
>  		fp_dbg("reporting enroll result");
>  		fpi_drvcb_enroll_stage_completed(imgdev->dev, r, data, img);
>  		if (r > 0 && r != FP_ENROLL_COMPLETE && r != FP_ENROLL_FAIL) {
> +			/* the callback above can cancel enrollment */
> +			if (imgdev->action != IMG_ACTION_ENROLL)

It would be better to check this condition in upper if
Comment 6 Timo Teräs 2012-12-03 09:38:15 UTC
(In reply to comment #5)
> Comment on attachment 70954 [details] [review] [review]
> imgdev: fix cancelling of enrollment from stage_completed callback
> 
> Review of attachment 70954 [details] [review] [review]:
> -----------------------------------------------------------------
> 
> ::: libfprint/imgdev.c
> @@ +146,5 @@
> >  		fp_dbg("reporting enroll result");
> >  		fpi_drvcb_enroll_stage_completed(imgdev->dev, r, data, img);
> >  		if (r > 0 && r != FP_ENROLL_COMPLETE && r != FP_ENROLL_FAIL) {
> > +			/* the callback above can cancel enrollment */
> > +			if (imgdev->action != IMG_ACTION_ENROLL)
> 
> It would be better to check this condition in upper if

Should I just move the new if block above, or merge the new test as part of the existing if test?
Comment 7 Vasily Khoruzhick 2012-12-03 09:43:01 UTC
(In reply to comment #6)
 
> Should I just move the new if block above, or merge the new test as part of
> the existing if test?

I prefer merging, and it worth adding some comment here that fpi_drvcb_enroll_stage_completed() can change device action.
Comment 8 Timo Teräs 2012-12-03 09:54:23 UTC
Created attachment 70961 [details] [review]
imgdev: fix cancelling of enrollment from stage_completed  callback [v2]

Updated patch per review comments.
Comment 9 Vasily Khoruzhick 2012-12-03 10:21:46 UTC
(In reply to comment #8)
> Created attachment 70961 [details] [review] [review]
> imgdev: fix cancelling of enrollment from stage_completed  callback [v2]
> 
> Updated patch per review comments.

Shouldn't it be (imgdev->action == IMG_ACTION_ENROLL)?
Comment 10 Timo Teräs 2012-12-03 11:08:11 UTC
Created attachment 70963 [details] [review]
imgdev: fix cancelling of enrollment from stage_completed  callback [v3]

Duh. Fixed the obvious cut'n'paste problem as observed in review.
Note to self: never post patch one minute before going to lunch...
Comment 11 Vasily Khoruzhick 2012-12-03 11:10:51 UTC
(In reply to comment #10)
> Created attachment 70963 [details] [review] [review]
> imgdev: fix cancelling of enrollment from stage_completed  callback [v3]
> 
> Duh. Fixed the obvious cut'n'paste problem as observed in review.
> Note to self: never post patch one minute before going to lunch...

Looks OK
Comment 12 Bastien Nocera 2012-12-14 12:11:21 UTC
Comment on attachment 70953 [details] [review]
uru4000: fix cancelling of imaging from error callback

Review of attachment 70953 [details] [review]:
-----------------------------------------------------------------

OK.
Comment 13 Bastien Nocera 2012-12-14 12:12:31 UTC
Comment on attachment 70955 [details] [review]
Update AUTHORS

Review of attachment 70955 [details] [review]:
-----------------------------------------------------------------

Looks like this isn't UTF-8 though.
Comment 14 Bastien Nocera 2012-12-14 12:13:07 UTC
Comment on attachment 70963 [details] [review]
imgdev: fix cancelling of enrollment from stage_completed  callback [v3]

Review of attachment 70963 [details] [review]:
-----------------------------------------------------------------

Sure.
Comment 15 Bastien Nocera 2012-12-14 12:18:52 UTC
All pushed, thanks.


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.