Nice to have a complete kernel+userspace example. Thanks!
Robert Baldyga wrote:
> +++ b/tools/usb/aio_multibuff/host_app/test.c
> @@ -0,0 +1,122 @@
> +#include <libusb-1.0/libusb.h>
Please don't do this.
Please always #include <libusb.h> and call some combination of:
pkg-config --cflags libusb-1.0
pkg-config --libs libusb-1.0
in the host application Makefile to get the correct compiler and
linker flags.
This allows your example Makefile to compile the application
correctly across all platforms supported by libusb, and perhaps even
more importantly it allows the host application to be cross-compiled
simply by setting PKG_CONFIG_LIBDIR appropriately for the
cross-compile toolchain.
Using pkg-config solves a whole bunch of problems - but it only works
if it's actually used, so please do. ;)
> +int test_init(struct test_state *state) {
> + int ret, i;
> + ssize_t cnt;
> +
> + state->found = NULL;
> + state->ctx = NULL;
> + state->handle = NULL;
> + state->attached = 0;
> +
> + libusb_init(&state->ctx);
Please check the return value. Suggest:
if (libusb_init(&state->ctx) != LIBUSB_SUCCESS) {
..
Also, create an exit path for the case that libusb returns an error
in test_init() so that your example demonstrates how to properly free
all libusb resources.
The current flow makes errors bubble up to main() which does a
return 1; and will leak if the code is copypasted into a larger
program as opposed to being used as a standalone example.
But I may be exaggerating this problem if nobody copypastes.. ;)
> + if(libusb_kernel_driver_active(state->handle ,0)) {
> + printf("device busy.. deatching\n");
typo, detaching, but..
> + libusb_detach_kernel_driver(state->handle, 0);
> + state->attached = 1;
> + }
> + else
> + printf("device free from kernel\n");
> +
> + if(ret = libusb_claim_interface(state->handle, 0)) {
> + printf("failed to claim interface\n");
> + return 1;
> + }
..this construct is unneccessarily racy. I suggest to directly call
_claim_interface() after libusb_open() and on failure then call
_detach_kernel_driver() and after that _claim_interface() a second
time.
If the second claim also fails then give up, clean up, tell user to
sort it out and exit.
> + else {
> + printf("no devices found\n");
> + libusb_free_device_list(state->list, 1);
> + libusb_exit(state->ctx);
> + return 1;
> + }
> + return 0;
> +}
> +
> +/*
> + * test_exit - cleanup test program
> + */
> +
> +void test_exit(struct test_state *state) {
> + if(state->attached == 1)
> + libusb_attach_kernel_driver(state->handle, 0);
> + libusb_close(state->handle);
> + libusb_free_device_list(state->list, 1);
> + libusb_exit(state->ctx);
> +}
I would call libusb_free_device_list() within test_init() *after*
having called libusb_open() on the device. There's no real reason
to keep the device list within struct test_state.
//Peter
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
