Found thanks to a warning about implicit sign conversion.
The return value is the new reference count, not an IOReturn error code.
I'm not sure if this is copy paste, and the new ref count is
unimportant, so I just preserved the existing behaviour (but adjusted
the log message). The change only impacts debug logs anyway.
References #1414
- Sometimes just change a type to match where it's coming from
- Sometimes add an explicit cast (but only if it's guaranteed correct,
like a previous check that a signed value is positive before casting to
unsigned)
- For libusb_bulk_transfer() add an assert to be certain that the signed
return value is never negative.
Update API documentation to underline this fact.
Add similar assert in usbi_handle_transfer_completion().
- darwin: For parsing OS version, change scanf to use %u instead of %i
- xusb: Add additional range checks before casting
- xusb: Reverse some backwards count and size arguments to calloc, not
that it really matters
Closes#1414
This prevents concurrent access to the list by the event background
thread, which could still be processing a previous hotplug event and
having to modify the list.
Fixes#1366Fixes#1445Closes#1452
References #1406
Remove all parents with a single child remaining in parent tree.
This ensures that no parents of the direct parent of the device being
considered are left in the list, when appearing before their child(ren)
in the processing order of the context device list cleaning.
References #1452
References #1406
All checks are enabled except for those that cause any warning.
This is a starting point, some of the currently-suppressed warnings can
be fixed hereafter.
Closes#1434
Programs using the previous version may use the new version as drop-in
replacement, but programs using the new version may use APIs not present
in the previous one.
Signed-off-by: Tormod Volden <debian.tormod@gmail.com>
umockdev relies on the per-context log callbacks to do its checks
properly, and these are not called if ENABLE_DEBUG_LOGGING is defined
(configure --enable-debug-log).
Fixes#1449
Signed-off-by: Tormod Volden <debian.tormod@gmail.com>
When setting LIBUSB_OPTION_LOG_CB before the first libusb_init(),
the global callback is set.
However, after the first libusb_init() there is a default context, and
setting LIBUSB_OPTION_LOG_CB would only set this default context, and
not the global callback. There would be no way to set the global
callback through libusb_set_option().
Change this so that the global log callback is set in addition to the
default context (when libusb_set_option is called with NULL context).
Closes#1397
But set it only on option LIBUSB_OPTION_LOG_CB.
Otherwise the following second call resets the callback to the
default one:
libusb_set_option(NULL, LIBUSB_OPTION_LOG_CB, cb_func);
libusb_set_option(NULL, LIBUSB_OPTION_LOG_LEVEL, LIBUSB_LOG_LEVEL_DEBUG);
References #1397
- Replace some uses of the soft-deprecated libusb_init() and
libusb_set_debug() with their updates.
- Updated various comments to favour use of the new functions.
- Notably remove \deprecated doxygen statement, since it causes
-Wdocumentation compiler warnings.
- Notably update the xusb example to pass debug options to
libusb_init_context() instead of setting environment variable.
- Also improve comments related to LIBUSB_API_VERSION and
LIBUSBX_API_VERSION.
Closes#1408
[Tormod: Fix Doxygen formatting and references]
Signed-off-by: Tormod Volden <debian.tormod@gmail.com>
They aren't included in any target, so they are never compiled.
But having them in the project allows doing global searches that also
look in these files.
Closes#1421
Also zero-initialize locationID since otherwise it could be used
uninitialized.
Also change the return variable to bool, matching what the function
actually returns.
Fortunately, this only affected log messages.
Closes#1412
[Tormod: Use PRIx32 for printing the 32-bit locationID]
Signed-off-by: Tormod Volden <debian.tormod@gmail.com>
The libusb.info website, like most sites these days, auto-redirects to
the https version anyway.
Also replaced a couple of other http links to https, after verifying
that they autoredirect to https.
Note this changes the "describe" field in the version info returned by
libusb_get_version(). This field is only there for ABI compatibility and
contains the website URL.
Closes#1407
In 2013, commit 858b794c added a comment saying "This function gets
called with the flying_transfers_lock locked".
That appears to have changed with 138b661f. Commit 88778414 improved
some stale comments, but missed these.
It is clear in the code that the comment is no longer true. The function
is *not* called with flying_transfers_lock locked, but it does lock
itransfer->lock.
References #1410
This is now symmetric with add_to_flying_list(), which was already
requiring that *callers* lock flying_transfers_lock. Updated the only
2 callers.
This also makes the code correspond better to the big comment about
locking made in 138b661f.
Also documented at the top of several functions when they require that
flying_transfers_lock already be held. Verified by code review that it's
actually the case.
This should not change any behaviour at all.
References #1410
This reflects the C99 terminology, instead of the old hack of using a
zero sized array.
Also adds the LIBUSB prefix to avoid namespace colisions, as this is
present in a public header.
References #1409
The `active_contexts_list` is supposed to be protected by the
`active_contexts_lock` mutex.
Upon code review, found one place where the mutex was not acquired.
Closes#1413
libusb_set_option() is a variadic function, so the type of the arguments
is not clearly defined. When called with LIBUSB_OPTION_LOG_LEVEL, the
argument is read with va_arg() as an int, which matches the type used
when passing constants, and also most of the internal calls and the
calls in the examples.
However the internal call site in libusb_init_context() passes the ival
element of the libusb_init_option struct directly, which is of type
int64_t. This breaks on big-endian architectures like PowerPC, as
detected by tests/set_option.
Therefore change the libusb_init_option struct to use int here as well.
Thanks to Aurelien Jarno for reporting and initial patch.
Closes#1416Closes#1436
Signed-off-by: Tormod Volden <debian.tormod@gmail.com>
After being suspicous of some code I was browsing, I tried changing
PTR_ALIGN to align to 4096 bytes instead of just to pointer size (8
bytes), then enabled ASan then ran the xusb example, and it crashed.
What ensued was a big code review of that function and related code.
- reviewed use of macros like USBI_TRANSFER_TO_LIBUSB_TRANSFER
- introduced new macros TRANSFER_PRIV_TO_USBI_TRANSFER and
USBI_TRANSFER_TO_TRANSFER_PRIV to do pointer offset conversions
- introduced some temporary variables, especially for
USBI_TRANSFER_TO_LIBUSB_TRANSFER results
- move some variable assignment and declaration together
- replaced a few uses of PTR_ALIGN to instead use the alignment macros
In particular, libusb_alloc_transfer() was not using PTR_ALIGN()
on struct usbi_transfer and could allocate a wrong size.
Closes#1418
Calculate the correct size of both the source and destination buffers,
then determine which is shorter, and iterate only that many times.
The original code would read one byte beyond the descriptor buffer
if a device returned a broken string descriptor of byte length 255.
Closes#1432
Commit 13a69533 slightly/subtly made an incorrect change to error
checking. Before that commit, we had
if (kIOReturnSuccess != kresult || !plugInInterface) {
return NULL;
}
which was correct. The commit changed the function signature. Instead of
returning the pointer directly, it now returns an error code directly,
and the pointer by reference. The above block became:
if (kIOReturnSuccess != kresult || !plugInInterface) {
return darwin_to_libusb(kresult);
}
But if kresult is somehow kIOReturnSuccess but plugInInterface is NULL
(probably impossible), then we'd return LIBUSB_SUCCESS but a NULL
pointer, which is a nonsense combination.
Closes#1430
Commit 13a69533 introduced a temporary variable for the device structure
(that originally was meant to be reverted, it turns out).
Here is a follow-up to revert this part, to avoid threading issues and
reported crashes.
The temporary variable would be harmless if there was no multithreading
happening, but there is:
On the hotplug background thread, darwin_devices_detached() is called,
which in turn calls Release() on the device, which frees the memory, and
then sets old_device->device to NULL. Shortly after,
darwin_devices_attached() is called (because the kernel driver is
reattaching?) and this calls darwin_get_cached_device(), which sets
->device to something new (and not NULL).
Meanwhile, back on the main thread, darwin_reenumerate_device() is
running and had cached ->device in the seemingly harmless temporary
variable. But that thing was already deallocated on the other thread!
Re-reading it from the structure makes it more likely you get the value
you want.
There might still be unfixed multithreading issues here, but this at least
avoids an obvious regression.
Fixes#1386Closes#1427
Some device descriptor fields are hard-coded by the HID backend, so they
will often not match. It is complicated to narrow this down to HID
devices, so we simply ignore these fields on Windows wholesale.
Hopefully we will fix the HID backend later, and this workaround can
then be reverted.
References #1360
References #1378
Signed-off-by: Tormod Volden <debian.tormod@gmail.com>
Depending on the devices or drivers, open() may return
LIBUSB_ERROR_ACCESS
LIBUSB_ERROR_NOT_FOUND
LIBUSB_ERROR_NOT_SUPPORTED
for devices "out of reach" to libusb.
References #1360
Signed-off-by: Tormod Volden <debian.tormod@gmail.com>
Instead of assuming there will always be some devices detected (when not
using LIBUSB_OPTION_NO_DEVICE_DISCOVERY), let the test pass but print a
warning to the user if there is none. This is to allow automated build
tests on restricted build environments without USB devices.
References #1374Closes#1379
In case num_devices equals 0, LIBUSB_EXPECT() calls
LIBUSB_TEST_CLEAN_EXIT(), which calls libusb_exit() on test_ctx which
has just been freed a few lines above by a call to libusb_exit(). This
might cause a SEGFAULT or SIGBUS depending on the system.
Fixes that by assigning NULL to test_ctx just after calling
libusb_exit() like it is done elsewhere in the file.
Closes#1374
Fixes the following warning from automake:
tests/Makefile.am:3: warning: 'LDFLAGS' is a user variable, you should not override it;
tests/Makefile.am:3: use 'AM_LDFLAGS' instead
Also, since stress_mt_LDFLAGS is set (even in a conditional), AM_LDFLAGS
will not be applied for stress_mt unless added explicitly.
Closes#1371
Signed-off-by: Tormod Volden <debian.tormod@gmail.com>
According to https://gcc.gnu.org/wiki/StaticAnalyzer the
-Wanalyzer-malloc-leak and -Wanalyzer-file-leak options came in GCC 10.
When building with GCC 9 there would be warnings:
CC umockdev-umockdev.o
../../libusb-git/tests/umockdev.c:37:32: error: unknown option after ‘#pragma GCC diagnostic’ kind [-Werror=pragmas]
37 | #pragma GCC diagnostic ignored "-Wanalyzer-malloc-leak"
| ^~~~~~~~~~~~~~~~~~~~~~~~
../../libusb-git/tests/umockdev.c:38:32: error: unknown option after ‘#pragma GCC diagnostic’ kind [-Werror=pragmas]
38 | #pragma GCC diagnostic ignored "-Wanalyzer-file-leak"
| ^~~~~~~~~~~~~~~~~~~~~~
Closes#1369
Signed-off-by: Tormod Volden <debian.tormod@gmail.com>
- Added long-awaited support for multithreading. Since WebUSB still
doesn't support accessing same device from multiple threads, this works
by proxying I/O operations to the main thread as discussed on the
original issue. For applications that run on the main thread nothing
changes and they will continue to access WebUSB via Asyncify like
before, while other threads will use blocking mechanism until an
asynchronous response is available from the main thread.
- Rewrote notification mechanism to use atomic waiting via sync Wasm
instructions or `Atomics.waitAsync` depending on the thread. This
results in simpler and faster notifications than the previous
`postMessage`-based approach (which was used because
`Atomics.waitAsync` wasn't yet available), as well as allows to send
notifications across threads for multithreading scenario described
above.
- Fixed notification access to only wait/notify on the event we're
interested instead of using a global lock.
- Rewrote descriptor reading to query device for raw device &
configuration descriptors instead of re-serializing them from
JavaScript object representation. This incurs slight extra cost for the
initial device opening, but fixes number of issues with information
that is not yet exposed via WebUSB and allows to read supplementary
descriptors such as string and BOS which were previously not supported.
- Fixed listing only one alternate instead of all the available ones.
- Fixed device closing & re-opening which could previously error out
with "device busy" error.
- Added mandatory Emscripten-specific linking flags to the generated
pkgconfig.
- Added device speed inference. This is not yet exposed via WebUSB, but
we can at least make a best effort guess based on USB version and
packet size, like some other backends already do.
- Simplified & fixed device session ID generation which is now
guaranteed to be truly unique, whereas previously it could clash with
other devices of the same type.
- Prepare code to support building for the Web without mandatory
multithreading (which is costly on the Web) in the future. For now
those `#ifdef`s are defunct as libusb is always compiled with
`-pthread`, but some upcoming changes on the Emscripten side will allow
to leverage those codepaths for smaller Wasm binaries.
- Require explicit `--host=wasm32-unknown-emscripten` as we might want
to add non-Emscripten WebAssembly backends in the future.
- Various smaller fixes and improvements.
Note that this requires Emscripten 3.1.48 or newer to build for some of
the features used here, namely `co_await` support for JavaScript values
(without it code would be both more complex and slower) and some
proxying APIs. It shouldn't be a big deal in practice as most users
retrieve Emscripten via the official emsdk installer or Docker images.
Closes#1339
This commit adds a new unit test that verifies that the interface
interface and device interface versions are as expected. The unit test
relies on new testonly symbols to get details about the implementation.
The commit also adds a sanity check on the versions to ensure that
libusb_init fails if a supported version can not be found.
In addition to the new tests this commit also updates the tests to
use the static version of libusb. This provides them access to any
hidden symbol including testonly symbols (which should never be
exported in the shared library).
Signed-off-by: Nathan Hjelm <hjelmn@google.com>
