Re: CVS commit: src/sys/dev/usb

[Manuel Bouyer <bouyer%antioche.eu.org@localhost>]

On Fri, Mar 16, 2012 at 02:15:29PM +1100, matthew green wrote:
> 
> > Module Name:        src
> > Committed By:       bouyer
> > Date:               Thu Mar 15 18:38:37 UTC 2012
> > 
> > Modified Files:
> >     src/sys/dev/usb: ehci.c
> > 
> > Log Message:
> > Do not read array before detecting array overflow, found by gcc -O3
> 
> hmmmm.
> 
> seems like this should move the DIAG code to just the printf().  this
> totally changes the behaviour of the failure mode as-is.
> 
> we can detect an overflow here but let non-DIAG code fail.

You mean, something like that:

Index: ehci.c
===================================================================
RCS file: /cvsroot/src/sys/dev/usb/ehci.c,v
retrieving revision 1.188
diff -u -p -u -r1.188 ehci.c
--- ehci.c      15 Mar 2012 18:38:37 -0000      1.188
+++ ehci.c      16 Mar 2012 07:50:28 -0000
@@ -2712,12 +2712,12 @@ ehci_alloc_sqtd_chain(struct ehci_pipe *
                        ehci_physaddr_t a = dataphys + i * EHCI_PAGE_SIZE;
                        if (i != 0) /* use offset only in first buffer */
                                a = EHCI_PAGE(a);
-#ifdef DIAGNOSTIC
                        if (i >= EHCI_QTD_NBUFFERS) {
+#ifdef DIAGNOSTIC
                                printf("ehci_alloc_sqtd_chain: i=%d\n", i);
+#endif
                                goto nomem;
                        }
-#endif
                        cur->qtd.qtd_buffer[i] = htole32(a);
                        cur->qtd.qtd_buffer_hi[i] = 0;
                }

I agree it would be better.
If you mean something else I don't understand :)

-- 
Manuel Bouyer <bouyer%antioche.eu.org@localhost>
     NetBSD: 26 ans d'experience feront toujours la difference
--