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>
The IOUSBDeviceInterface object is used by libusb to interact with USB
devices macOS (Darwin, MacOS X, iOS, etc). For as long as libusb has
existed it has always used the highest available version (at compile
time) of this interface. This change breaks that by using the higest
version available between the compile environment and the version of
macOS running at execution time. This allows the same libusb build to
run on older and newer versions of macOS without disabling newer
features.
This change relies heavily on the fact that each new version of the
IOUSBDeviceInterface object builds on prior versions. This means that
libusb can use the oldest version of the interface that has a specific
function without needing to use the specific version specified at open
time which greatly simplifies the code.
Signed-off-by: Nathan Hjelm <hjelmn@google.com>
The IOUSBInterfaceInterface object is how libusb interfaces with the
interfaces of USB devices on macOS (Darwin, MacOS X, iOS, etc). For as
long as libusb has existed it has always used the highest available
version of the interface. This change breaks that by using the higest
version available for the current running version of macOS. This allows
the same libusb build to run on older and newer versions of macOS
without disabling newer features.
This change relies heavily on the fact that each new version of the
IOUSBInterfaceInterface object builds on prior versions. This means that
libusb can use the oldest version of the interface that has a specific
function without needing to use the specific version specified at open
time which greatly simplifies the code.
This commit update IOUSBInterfaceInterface only and leave the
IOUSBDeviceInterface as-is. A follow-on change will update the device
interface.
Signed-off-by: Nathan Hjelm <hjelmn@google.com>
This helper returns an integer representation of the currently running
OS from 100000 (10.0) on. This allows the Darwin backend to make
decisions base on the running version without using the
__builtin_available() feature. This builtin is only available in Clang
and not gcc. For newer versions of macOS the helper uses the
kern.osproductversion sysctl and falls back to kern.osrelease and a
mapping of Darwin -> OS X/macOS versions.
Signed-off-by: Nathan Hjelm <hjelmn@google.com>
Move make check into the if conditional to ensure that the failure does
not force the script to immediately exit.
Signed-off-by: Nathan Hjelm <hjelmn@google.com>
The test output is hidden by default with make check so we need to explicitly
dump the test suite log on failure.
Signed-off-by: Nathan Hjelm <hjelmn@google.com>
automake has built-in support for running unit test with make check. This commit
sets the TESTS variable in tests/Makefile.am to enable this support and updates
the CI script to use make check instead of hard-coded test names.
Closes#1361
Signed-off-by: Nathan Hjelm <hjelmn@google.com>
Omit specifying bash shell, which is default on Linux (and macOS)
anyway, also since we are not using any particular shell features.
This makes it all consistent with the mockdev step.
Signed-off-by: Tormod Volden <debian.tormod@gmail.com>
It doesn't add much extra noise and helps a lot to interpret
the existing debug output.
Change the wording slightly from the previous commented-out
debug output. Grep for "ENUM" to find these in the logs.
Signed-off-by: Tormod Volden <debian.tormod@gmail.com>
Verify that you can open a device, do some transfer (a device descriptor
control transfer was chosen as it should succeed with any connected
device), and close a device from multiple threads in parallel as well.
Print a warning if a device is readonly.
Consistently add thread flags to the build.
Closes#1347
The logging function usbi_dbg() called from udev event thread needs to
safely get the log level of the default context or the fallback context.
This is not trivial because udev thread may log events while contexts
are created and destroyed. Locking the default context mutex is not an
option because usbi_dbg() may be called from code that already holds
this mutex, which would lead to a deadlock. The solution implemented in
this commit is to maintain a separate atomic global variable with the
default debug level.
The issue was found with ThreadSanitizer, although no actual bugs have
been observed.
libusb_set_debug() was also changed to call libusb_set_option() instead
of doing its own option setting which had fewer checks.
Closes#1332
This is only for readability and doesn't change the behaviour.
Several passes were implicitly grouped together in the "default" case,
which made the code unnecessarily hard to read. The reason was that the
number of EXT passes is not known, so those were conveniently treated in
the "default" case together with some non-EXT passes.
Instead introduce a "pass_type" helper variable which has a defined
range, and add all pass types explicitly in the switch statements.
At the same time update the comment which shortly explains the different
passes, it hasn't reflected reality for a long while.
Signed-off-by: Tormod Volden <debian.tormod@gmail.com>
This reverts commit cd8078db, which although it made a static analyzer
happy, unfortunately got compilers to complain (with -Wswitch-enum
enabled).
Signed-off-by: Tormod Volden <debian.tormod@gmail.com>
set_option.c gave warnings and init_context.c would fail to build on
Windows.
Tested with configure options --disable-log and --enable-debug-log
Signed-off-by: Tormod Volden <debian.tormod@gmail.com>
