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

_______________________________________________
[email protected] mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-usb
To unsubscribe, send any mail to "[EMAIL PROTECTED]"

Reply via email to