Date: Tue, 15 Jan 2008 18:37:57 +0200 From: Stefan Lambrev <stefan.lambrev@moneybookers.com> To: freebsd-usb@freebsd.org Subject: Re: Problem with usb4bsd rev566 Message-ID: <478CE165.7060001@moneybookers.com> In-Reply-To: <478CBCDB.6090203@moneybookers.com> References: <477BC1A3.5080406@moneybookers.com> <200801021837.51376.hselasky@c2i.net> <477CD474.6080302@moneybookers.com> <200801032154.32107.hselasky@c2i.net> <478BE02A.7050100@moneybookers.com> <478CBCDB.6090203@moneybookers.com>
next in thread | previous in thread | raw e-mail | index | archive | help
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?478CE165.7060001>