Problem with usb4bsd rev566
Stefan Lambrev
stefan.lambrev at moneybookers.com
Tue Jan 15 08:38:03 PST 2008
Greetings,
Sorry to reply again to myself :)
Stefan Lambrev wrote:
> -cut-
>
> cc -c -O2 -pipe -fno-strict-aliasing -march=prescott -std=c99 -Wall
> -Wredundant-decls -Wnested-externs -Wstrict-prototypes
> -Wmissing-prototypes -Wpointer-arith -Winline -Wcast-qual -Wundef
> -Wno-pointer-sign -fformat-extensions -nostdinc -I. -I/usr/src/sys
> -I/usr/src/sys/contrib/altq -D_KERNEL -DHAVE_KERNEL_OPTION_HEADERS
> -include opt_global.h -fno-common -finline-limit=8000 --param
> inline-unit-growth=100 --param large-function-growth=1000
> -mno-align-long-strings -mpreferred-stack-boundary=2 -mno-mmx
> -mno-3dnow -mno-sse -mno-sse2 -mno-sse3 -ffreestanding -Werror
> /usr/src/sys/dev/usb/usb_transfer.c
> cc1: warnings being treated as errors
> /usr/src/sys/dev/usb/usb_transfer.c: In function 'usbd_callback_intr_td':
> /usr/src/sys/dev/usb/usb_transfer.c:2094: warning: 'xfer[2]' may be
> used uninitialized in this function
> /usr/src/sys/dev/usb/usb_transfer.c:2094: warning: 'xfer[3]' may be
> used uninitialized in this function
> *** Error code 1
>
> Stop in /usr/obj/usr/src/sys/CORE.
> *** Error code 1
>
> Stop in /usr/src.
> *** Error code 1
>
> Stop in /usr/src.
>
I think I got it, why the compiler complains and refuses to build this.
What we have in usbd_callback_intr_td() is little strange "goto" cycle
(at least strange for me):
repeat:
if (xfer[0]) {
LIST_REMOVE(xfer[0], done_list);
xfer[0]->done_list.le_prev = NULL;
xfer[1] = LIST_FIRST(&(info->done_head));
if (xfer[1] == NULL) {
dropcount = 1;
goto lockchange_0;
}
At this point xfer[2] & xfer[3] are not defined and if xfer[1] == NULL
we go here:
lockchange_0:
mtx_unlock(info->usb_mtx);
/*
* we exploit the fact that the mutex is the same for
* all callbacks
*/
mtx_lock(info->priv_mtx);
/* call callback(s) */
usbd_callback_wrapper(xfer[0], info, USBD_CONTEXT_CALLBACK);
Here we already know that xfer[1] == NULL (but the compiler does not!)
and I think the logic can be changed to remove the second check
if (xfer[1] == NULL) {
goto lockchange_1;
}
usbd_callback_wrapper(xfer[1], info, USBD_CONTEXT_CALLBACK);
We can reach to this point if xfer[1] == NULL, but the compiler sees
reference to xfer[2] so it complains - may be used uninitialized in this
function
if (xfer[2] == NULL) {
goto lockchange_1;
}
usbd_callback_wrapper(xfer[2], info, USBD_CONTEXT_CALLBACK);
Same situation with xfer[3] !
if (xfer[3] == NULL) {
goto lockchange_1;
}
usbd_callback_wrapper(xfer[3], info, USBD_CONTEXT_CALLBACK);
What I did to comfort the compiler is to add
xfer[2] = NULL;
xfer[3] = NULL;
On the next line after label "repeat" (should work better if added
before the label?)
As I said more adequate patch is to fully remove lockchange_0 label and
do all things in the first check.
It will be more optimized and more readable :)
I'll test what I did and do I still have working usb and will report
back ASAP ;)
P.S. usbd_callback_intr_td() is declared and called only in
i4b/trunk/i4b/src/sys/dev/usb/usb_transfer.c
--
Best Wishes,
Stefan Lambrev
ICQ# 24134177
More information about the freebsd-usb
mailing list