Hi, On 05/29/2013 02:36 AM, Pete Batard wrote: > Finally got a chance to check out the updated branch.
Many thanks for looking at it! > First, the usual stream of compilation errors: > > 1. cygwin is being an ass (as usual), with: > sam3u_benchmark.c: In function ‘benchmark_in’: > sam3u_benchmark.c:100:5: warning: passing argument 7 of > ‘libusb_fill_iso_transfer’ from incompatible pointer type > ../libusb/libusb.h:1568:20: note: expected ‘libusb_transfer_cb_fn’ but > argument is of type ‘void (*)(struct libusb_transfer *)’ > sam3u_benchmark.c:104:5: warning: passing argument 6 of > ‘libusb_fill_bulk_transfer’ from incompatible pointer type > ../libusb/libusb.h:1511:20: note: expected ‘libusb_transfer_cb_fn’ but > argument is of type ‘void (*)(struct libusb_transfer *)’ > > I'm attaching the patch to fix this, which I hope can be integrated with the > sample addon commit during integration. Thanks, patch squashed into the original commit, I'll do a (forced) push after this mail to get all your fixes out there. > 2. WDK also produces the following: > 1>d:\libusbx-hans\libusb\descriptor.c(339) : error C2220: warning treated as > error - no 'object' file generated > 1>d:\libusbx-hans\libusb\descriptor.c(339) : warning C4242: '=' : conversion > from 'int' to 'uint8_t', possible loss of data > 1>d:\libusbx-hans\libusb\descriptor.c(445) : warning C4242: '=' : conversion > from 'int' to 'uint8_t', possible loss of data > 1>d:\libusbx-hans\libusb\descriptor.c(482) : warning C4242: '=' : conversion > from 'int' to 'uint8_t', possible loss of data > 1>d:\libusbx-hans\libusb\descriptor.c(880) : warning C4242: '=' : conversion > from 'int' to 'uint8_t', possible loss of data > > Patch attached (went for the simplest but maybe we want to switch to using an > uint8_t rather than an int for i). "Happy are those who need compile on one > platform only, for they shall have much time to spare after supper..." Thanks, I went with the patch as is. > 3. With its gettimeofday(), unistd.h and sigaction, sam3u_benchmark is not > compatible with native Windows at all, i.e. MSVC and WDK are out (and likely > MinGW too). Therefore I'm gonna have reservations including that sample, > until we can make it work on all the main platforms. > While it was acceptable for samples that were crafted before the Windows > backend inclusion not to be included in MSVC/WDK (such as dpfp), I'm not too > happy about samples that were crafted post to still be platform specific. > We're trying to demo a cross-platform library here, so I think it makes sense > to want new samples to be cross-platform too. I'm not against dropping sam3u_benchmark the only reason I added it is: "sync everything which looks like it may be useful from libusb git to libusbx git" So I see 2 ways forward keep it and try to make it more portable one of these days, or at a git commit reverting it explaining why we've decided to not keep it. I do believe having a commit adding it + revert is the right thing to do wrt trying to merge the 2 gits back into one and keeping as much history as possible. > Apart from that, and as far testing and compilation go on Windows, things > look OK. We do get an extra warning in the new Code Analysis tool from MSVC > (we already had 2), which doesn't surprise or bother me too much as the MS > tool can't quite grasp that yes there may be separate but companion > functions, to handle the creation and deletion of descriptor data, and where > we're fairly positive that null derefs won't happen. Good glad to hear that! > Now, for additional non-compilation remarks: > > * There's a bunch of whitespace issues in > https://github.com/jwrdegoede/libusbx/commit/2da19ff1ada7ea00f26645d7094e6b2525b0d829#L0R131 > where tabs should be used instead of spaces. Good catch, thanks! Fixed (and fixes squashed into the original commits). > * With the new BOS/ep_comp structs, and zero sized arrays, don't we want to > also go with the heavy but present everywhere else: > #if defined(__STDC_VERSION__) && (__STDC_VERSION__ >= 199901L) > [] /* valid C99 code */ > #else > [0] /* non-standard, but usually working code */ > #endif > ? And another good catch, also fixed (and fixes squashed into the original commits). > > * We have quite a lot of libusb-1.0 references all over the place > (libusb-1.0.lib in much of the MSVC project files, libusb-1.0.def, > libusb-1.0.pc.in, etc.). Don't we want libusb-1.2 everywhere? > If the answer to that is "On POSIX, we don't" then I guess I will go back on > my vote for the next release to be 1.2, and instead push to stick to 1.0 > until we are actually ready to update the minor. The last thing I want is > have to spend the next few years explaining to Windows users, who don't > necessarily have a clue that on POSIX library versioning and source > versioning don't necessarily follow, that, yes, the libusb-1.0.dll they are > using is *really* v1.2... Hmm, so on posix we only care about the libusb-1.0.pc.in file for the files you mention. That one definitely needs to stay libusb-1.0.pc.in, since we implement an extended version of the 1.0 API, and apps configure scripts will contain things like pkg-config --libs 'libusb-1.0 >= 1.0.15' And that should stay working with the 1.2 releases, this is quite normal, ie: [hans@shalem ~]$ rpm -qf /usr/lib64/pkgconfig/gtk+-2.0.pc gtk2-devel-2.24.18-1.fc19.x86_64 [hans@shalem ~]$ rpm -qf /usr/lib64/libgtk-x11-2.0.so.0 gtk2-2.24.18-1.fc19.x86_64 IOW gtk2 2.24 still provides a gtk+-2.0 pkg-config file and .so files: If you would prefer for the windows dll to be called libusb-1.2.dll that is fine with me, although I guess this means adding some conditionals for the mingw build. Or we could drop my "all: The next release will be 1.2.0 not 1.0.16" commit and simply go with 1.0.16. Unless we've some agreement on this beforehand I'll drop that commit before I push things to libusbx master tomorrow, so that will be 1.0.16 for now then, and we can always add that commit + fixes later to make it 1.2.0 > If we're going to be non-intuitive for a whole set of platform developers (in > the vein of "system32 is for 64 bit apps, whereas SysWOW64 is for 32 bit > ones"), I'd rather avoid the non-intuitiveness of one platform being exported > across the board => either we replace every last 1.0 we have with 1.2 (and > yeah, that'll mean devs having to update their linking and stuff), or we > stick to 1.0. > > Now, if we want to go 1.2 everywhere, judging by what a search on libusb-1.0 > returned, we still have quite a lot of work ahead of us... Going 1.2 everywhere is not really an option, at-least not for the .pc and .so files under POSIX platforms, doing it elsewhere is fine with me. > On 2013.05.27 15:45, Hans de Goede wrote: >> Working towards that goal I've added BOS and SS EP comp support to my tree >> today. It takes all things discussed previously into account. One new thing >> is that I've renamed the libusb_bos_attributes enum to >> libusb_usb_2_0_extension_attributes >> and added a libusb_ss_usb_device_capability_attributes enum, as the 2 are not >> the same. The USB 2.0 Extension descriptor has a LPM bit, where as the >> SuperSpeed USB Device Capability descriptor has a TPM bit, close, but >> definitely not the same! > > Sounds good to me. xusb checks against an FX3 device looked fine too. Btw, > when did we add topology to listdevs? That looks real neat! I did that while testing Nathan's hotplug + topology fixes, so as to have something which actually used the topology stuff: https://github.com/jwrdegoede/libusbx/commit/ebac6ac1b30aea6e9190cce336eb75dc0f1c5ce7 :) Regards, Hans ------------------------------------------------------------------------------ Introducing AppDynamics Lite, a free troubleshooting tool for Java/.NET Get 100% visibility into your production application - at no cost. Code-level diagnostics for performance bottlenecks with <2% overhead Download for free and get started troubleshooting in minutes. http://p.sf.net/sfu/appdyn_d2d_ap1 _______________________________________________ libusbx-devel mailing list libusbx-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/libusbx-devel
