unsigned integers, which is what libusb uses everywhere.
As it was before, there are parts of the library that return bytes & shorts, like the Descriptors (mostly to
be compatible with javax.usb), and others that take and return int, even where an 8 or 16-bit quantity would
be expected by libusb. I suspect this was done to avoid casting and try to side-step the sign-extension
problem with Java as much as possible, but in the end it leads to an API that's inconsistent, that doesn't
express the ranges it can use well (it's an int but in reality only 16 bits for libusb and so on). It also
fails at basic operations in ways you'd not expect because of the above inconsistencies:
if (deviceDescriptor.bDeviceClass() == LibUsb.CLASS_VENDOR_SPEC) { do something; }
will never work, since the constant is an int and the returned short from bDeviceClass() will be cast up to an
int too, but CLASS_VENDOR_SPEC is 0xFF (MSB=1), it will be sign-extended and the comparison will fail always.
There are two solutions I can see here:
A) consistently use int/long everywhere, both input and output. Everything becomes simpler to write, with less
casts, BUT javax.usb compatibility is gone for the Descriptors and it becomes even unclearer what those values
effectively mean and what range they support.
B) restrict the API to use the proper integer sizes as the C API, ie. 8-bit quantities (char, uint8_t, int8_t)
are represented by jbyte, 16-bit by jshort, and all the constants get properly sized too. With this approach
the sizes and ranges are much clearer, comparisons like the above work out-of-the-box, javax.usb compatibility
is maintained for the Descriptors. The downside is more casting and the occasional need to mask off returned
values to kill the sign-extension when you want to use those values as integers (like when printing stuff in
decimal notation).
I chose to try out the approach from B) for now, this commit implements it as explained above.
It requires minimal changes to the javax.usb implementation, just two casts to byte in AbstractDevice.java.
It would be counter-intuitive to have to set negative timeouts in Java to get longer timeouts in C...
Also reorder getVersion() and getStringDescriptorAscii() to their proper places, so they are in the same order
as they appear in the libusbx documentation.
Enable silent building and C99 support.
Enable lots of warnings and don't enable Werror, as there are warnings in libusb.h.
Fix all warnings. Add const to exceptions.
Make it possible to pass null to setDevHandle and setBuffer in Transfer now.
Make sure that returning a new reference always leads to a new Java object being created, else
freeing/unref'ing that one would lead to the reset of the original object too.
Furthermore, ensure that getDevice also increments the reference count, since if you then free it,
which is usual and not forbidden (libusbx docs just tell you are not obligated to unref it), the
reference count would be wrong and lead to a segfault. This way it would lead at most to keeping a
device around, instead of segfaulting. Still, always free your references!!!
Fix submitted to libusbx as pull request #116.
Also fix this inside the Java wrapper, by adding our own refcount (similar to what was in original usb4java,
but allowing multiple calls with the default context), and making the access to said variable thread-safe.
Once libusbx fixes this, the above code can be removed.
This is obviously unexpected and incorrect, yet cannot be fixed by just adding more data on the C side: you'd have to remember the callback object at the very least, which you could in the user_data
field, but you'd have to make it a global reference. Yet, there would be no easy way to later correctly delete that global reference, since to unset the pollfdNotifiers you just pass NULL to the
same function again, and you don't get back any data on what was there before.
The easiest way to fix this was thus to change to a HashMap, Concurrent since access can be from random threads.
The key was chosen to be the Context.hashCode(), as Integer lookup and comparison is fast, and the hashCode depends directly on the contextPointer stored in the Context (which identifies it).
This works for sure on 32bit systems, but I'm still concerned about 64bit systems, where a long would probably make more sense, I need to think about it some more.
Everything is done in Java here as it's much easier and cleaner, since those are only convenience functions/structs.
Byte order conversion is conveniently handled by ByteBuffer too, and using slice() we access the same memory all around.
Added some final keywords to LibUsb and BufferUtils.
Only fill_control/get_control is still outstanding.
getDescriptor() and getStringDescriptor() were removed from native JNI, since they are static inline convenience functions in libusb.h and don't do anything more than calling the appropriate real function.
To support FREE_TRANSFER in the case the user doesn't set (or explicitely sets) a callback of null on the Java side, a small C-callback was created that just takes care of cleaning up the Java objects and resources.
Add methodID field to transfer_data structure, so that it can be cached at set() time and the callback is faster.
Also update the length when setting a new buffer!
Check all variable names for consistency.
Fix memory leak on failed getDeviceDescriptor.
Use result == LIBUSB_SUCCESS instead of just !result, it is much clearer what the integer that you get as result actually means that way.
Move callback and maxNumIsoPacketSize to C, minimizing what is kept in Java: only the pointer and the transferBuffer. setLength() is still checked here, as that's easiest.
Update the tests to match the changes and add documentation on the new methods.
*.java: Fix code and comment formatting to be equal everywhere and conform to what I could see as being the
expected usb4java standard.
*.java: Remove Apache-Commons dependency for extremely simple HashCode/Equals involving only the pointer
variable, instead use the Eclipse-generated methods.
calls return without throwing exceptions.
DescriptorUtils.java: update to use the correct masks and variables, always return "Unknown" on a not-handled
value.
*Descriptor.java: update equals/hashcode to use all fields in the same order as they are defined (makes
checking this much easier in the future!). For example bRefresh was missing or some extra/extraLength checks.
InterfaceDescriptor: there is no need for a dump(DeviceHandle) here, so removed it and updated the call in
Interface.java.
since you don't know if the refcount reached zero and it was deallocated. Every new reference means the
refcount gets increased AND a new reference indeed exits, so this always works, unless you let references go
out of scope or overwrite them, which is clearly incorrect in any case.
Also add missing checks for setDeviceHandle() in Transfer.
Right now for example, getting a new config descriptor re-using an old ConfigDescriptor object just overwrites the old pointer, without freeing it or telling the user, resulting in a memory leak.
This way, the user gets notified when he's re-using an object that he hasn't cleaned up yet, and he can react and call the appropriate free/cleanup function.
Add missing NOT_NULL checks.
Simplify getPortPath() ifdef.
Check for result value in libusb_init and only setContext if the call succeeded.
Use always same style of NULL checks in usb4java.h.
Also, I don't see why there is any need to deviate from using a pointer to using a ByteBuffer here, it will not make memory management automatic, and just makes the DeviceDescriptor "different" for no apparent reason. So changed back to pointer.
