When a new connection came in we would close the old connection. This in
turn would trigger a receive error causing the *new* connection to be
closed from the error handler.
Fix this by simply cancelling any pending transfers when a new
connection comes in. Also change the error handling code to catch issues
like partial writes correctly.
This fixes an issue for the fprintd test where some tests were flaky.
It is not very useful to just delete the data again after a failure, as
it might be useful for debugging. Just store it into an "errors"
subdirectory of the PWD in the hope that this is a sane location.
Note that it'll error out if the directory already exists, but that
should be acceptable in all cases. i.e. it won't exist in the CI and
developers can just wipe the directory.
When porting the driver to the new libfprint 1.90.0 a mistake was made
where the device was not passed through user_data anymore but it was
still read from there. Stop using user_data in the callback to fix this.
See: #320
The elan driver always "deactivates" the device after a cpature run. We
can simplify the while internal state into a single "active" state and
rely on the image device driving this state machine correctly.
i.e.:
* We start a callibrate/capture/deactivate when we go into the
AWAIT_FINGER_ON state
* We only store the fact that we are active and want to deactivate
* We rely on the image device never going into the AWAIT_FINGER_ON
state without first waiting for the finger to be off (which implies
deactivation).
This adds a number of new internal states to better capture what is
going on. Also added are checks that all transitions we make are in the
set of expected and valid transitions.
Only three drivers use the state_change notification. These drivers are
updated accordingly.
The boolean is just used to emit a warning for unexpected state
transitions. It is sufficient to pass it to the deactivate function
directly for that purpose.
We might redo image transfers, but we only ever had one reference that
was implicitly removed after the transfer completed. Add a new reference
each time it is submitted and only free the last reference in the stop
handler.
For some reason we re-submitted the interrupt transfer with a timeout of
1s while the original submission was without a timeout. This looks like
typo, remove the timeout.
Annotate the USB transfer creation functions with the correct buffer
access attribute. Note that we only annotate them as read_only as the
functions may be used for sending and receiving.
Hopefully this will catch buffer overflows in drivers in the future.
Doing this avoids race conditions in testing code where the code might
try to submit errors/images before the device has been activated. This
would then (sometimes) trigger assertions in the image driver code.
The earlier image device code tried to hide deactivation from the
surrounding library. However, this does not make any sense anymore with
the early reporting feature present.
This changes the operation to complete only once the device is
deactivated. Also changed that in most cases (except for cancellation)
we wait for the finger to be removed before deactivating the device.
The driver seems to have relied on the device to be fully
deactivated and activated again for each capture. This is not the case
during enroll, and as such in can cause issues.
Try fixing this by splitting out the last few commands send to the
device and assuming that this starts the capture.
Fixes: #306
We have plenty of code paths where a transfer may be cancelled before it
is submitted. Unfortunately, libgusb up to and including version 0.3.6
are not handling that case correctly (due to libusb ignoring
cancellation on transfers that are not yet submitted).
Work around this, but do so in a somewhat lazy fashion that is not
entirely race free.
Closes: #306
Delay the open/close callbacks by 100ms so that we have a window of
opportunity for race conditions. This is needed to test certain
conditiosn in fprintd.
Adding a trailing \n to g_message, g_debug, g_warning and g_error is not
neccessary, as a newline will be added automatically by the logging
infrastructure.
The cancellable needs to be free'ed at deactivation. Also free it if we
run into a fatal error, which then in turn indicates that the device is
deactivated already.
The change_state function is called synchronously from the
image_captured callback. This means that deactivation of the device
happens during the img_cb function, causing the USB transfer to be
re-registered even though the device is already deactivating.
There are various ways to fix this, but it makes sense to directly bind
the cancellation to the deactivation. So create a cancellable that we
cancel at deactivation time, and make sure we always deactivate by going
through cancellation.
closes: #306
Image devices are simply deactivated suddenly. As such, the cancellation
logic of FpDevice is not really useful there, but the documentation was
apparently accidentally copied unmodified.
This is unlikely to happen in a real world scenario and currently breaks
running the CI test (not the umockdev based ones) while building the
flatpak. Lower the severity to avoid aborting because there is a
warning.
This is based on the patch and observation from Bastien that some
URU4000B devices do not use encryption by default (it is a configuration
stored within the firmware). As such, it makes sense to always detect
whether encryption is in use by inspecting the image.
The encryption option would disable flipping of the image for the
URU400B device. Retain this behaviour for backward compatibility.
The code would just read 4096 bytes from the packet, without checking
the size and neither setting short_is_error. It is not clear whether
packets from the device are always 4096 bytes or not. But the code
assume we always get a full line, so enforce that and use the actual
packet size otherwise.
This was not used. The old driver used this if creating a USB transfer
failed, however, we delay any such failures (which cannot really happen)
into the callback today, where the error is handled differently.
The GPtrArray needs to be created at some point. Also, reference
counting was wrong as submitting the transfer sinks the ref, but we rely
on it surviving.
Note that we really should change this to only have one in-flight
transfer and starting a new one after it finishes.
Co-authored-by: Vasily Khoruzhick <anarsoul@gmail.com>
When sending static data, it would not be copied. The function that
sends it assumed that it should be free'ed though.
Fix this by simply always making a copy.
The driver would warn about the fact that a state change is queued, but
still queue it a second time. This would result in deactivation to run
twice.
See: #216
The image driver may still be deactivating when a new activation request
comes in. This is because of a hack to do early reporting, which is
technically not needed anymore.
Fix the immediate issue by properly reporting the retry case. The proper
fix is to only finish the previous operation after the device has been
deactivated.
One can set it in the project, but that doesn't get copied to forks. And
that means the coverage information isn't printed in MRs sometimes.
Just add it into .gitlab-ci.yml so that it always works.
The driver might forget to set the type of the print. Catch that error a
bit earlier rather than failing when the API user tries to load it from
disk again.
The type of the print (RAW or NBIS) needs to be filled in by the driver.
For most drivers the image devices does this (NBIS), but the
corresponding call was missing in the upekts driver, rendering the
enrolled print unusable.
__handle_incoming_msg would copy the payload of the message into a newly
created buffer just to destroy it again immediately after calling the
callback. Just reference the correct address inside the original package
instead.
Also, in one case the extra buffer was leaked afterwards.
The driver would correctly calculate the amount of extra space needed to
receive the whole packet. It would also request the correct number of
bytes for this transfer.
However, the reallocated buffer to hold this data was directly derived
from the expected payload size and did not include the overhead.
Make the code more explicit and get rid of the confusing
MAX_DATA_IN_READ_BUF define that hides details on buffer allocation
calculation from the code.
The device is already beeing de-initialised from the verify/enroll
commands. Trying it again will result in a timeout error as it is not
responding properly at that point.
Not having the diff is a bit painfull when the local version of
uncrustify differs from the one on the CI runner. So uncrustify in-place
and output the diff.
We build the HTML documentation. For feature branches, it is convenient
to be able to view the documentation easily. Expose them as artifacts
and add a link to the browser underneath the pipeline in the MR.
Unfortunately, it does not seem to be possible to link directly to the
HTML.
For some reason static checkers report that the auto-free is not run if
the goto exists the loop. It seems to me like that should work fine, but
we can simply make the analysers happy by moving it.
We need more than 1 line for assembling, but in general, we should
have a reasonable amount of lines. So use the width in the hope we'll
get an image that is about square at least.
Closes: #135
The minutiae detection might fail and we must not copy any data at that
point. So check g_task_had_error to ensure that we only do so when the
task was successful.
Fixes: #251
It appears the order of linking is relevant in this case, change it to
fix some linking issues.
It may be that there are better solutions to this problem.
Without this we get warnings like the following:
cc1: warning: .../_build/libfprint/nbis/libfprint-include: No such file or directory [-Wmissing-include-dirs]
We are already using a number of defines and autoptrs from newer GLib
releases. Add the appropriate compatibility defines rather than removing
the corresponding code.
Closes: #222
The callback function would continue processing even after having failed
the SSM already. This causes further invalid operations on the SSM.
This error was found using a coverity scan.
The g_object_new call had a NULL argument for a property. This meant
that the compiler could not warn about the lack of NULL termination for
the argument list.
Add the missing NULL termination.
We were not testing the image device error reporting functions yet
inside libfprint (fprintd already had such tests). Add tests to make
sure we catch errors earlier.
Benjamin Berg