The following reply was made to PR port-mac68k/32583; it has been noted by GNATS.

From: khym@azeotrope.org
To: Hauke Fath <hauke@Espresso.Rhein-Neckar.DE>
Cc: Scott Reynolds <scottr@clank.org>,
	port-mac68k-maintainer@netbsd.org, gnats-bugs@netbsd.org
Subject: Re: port-mac68k/32583: mac68k netbsd-2 panics during rcp(1)
Date: Sun, 3 Sep 2006 18:44:12 -0500

 On Sun, Sep 03, 2006 at 08:54:48PM +0200, Hauke Fath wrote:
 > Your patch and the last one that Scott provided differ only in that
 > 
 > >Index: if_ae.c
 > >===================================================================
 > >RCS file: /cvsroot/src/sys/arch/mac68k/dev/if_ae.c,v
 > >retrieving revision 1.77
 > >diff -u -r1.77 if_ae.c
 > >--- if_ae.c	11 Dec 2005 12:18:02 -0000	1.77
 > >+++ if_ae.c	3 Sep 2006 08:27:17 -0000
 > >@@ -165,15 +165,18 @@
 > > 		}
 > > 	}
 > >
 > >+	len = ETHER_MIN_LEN - ETHER_CRC_LEN - totlen;
 > > 	if (wantbyte) {
 > > 		savebyte[1] = 0;
 > > 		bus_space_write_region_2(sc->sc_buft, sc->sc_bufh,
 > > 		    buf, (u_int16_t *)savebyte, 1);
 > >-		    buf += 2;
 > >+		buf += 2;
 > >+		if (len-- > 0)
 > >+			totlen++;
 > 
 > his has an additional
 > 
 > 	        len--;
 > 
 > here.
 
 Hmm, Scott's patch has:
 
 	if (len-- > 0)
 		totlen++;
 	len--;
 
 rather than
 
 	if (len > 0)
 		totlen++;
 	len--;
 
 ? ("len--" vs. "len" in the first line) The latter would be identical to
 my patch (and is perhaps easier to read). The former seems like it would
 cause problems with short packets not being padded enough. For example,
 consider a packet that's 57 bytes long (ETHER_MIN_LEN - ETHER_CRC_LEN -
 3), or 3 bytes too short. The main loop will have written 56 bytes,
 leaving the last byte stored in savebyte[0] for the if (wantbyte) section
 to output. len will have been set to 3 by the assignment immediately
 before the if (wantbyte). So, the if (wantbyte) writes the last byte,
 plus a byte of padding. len will be decremented to 2, totlen incremented
 to 58, then len decremented again to 1. Then the if (len > 0) section
 will call bus_space_set_region_2() with a count of (len >> 1), which
 is 0. Which is not a valid value, and would cause the bus error that
 started this PR in the first place :)
 
 However, if that first line is "if (len > 0)", rather then "if (len--
 > 0)", it seems like everything should be fine. The main loop writes 56
 bytes, len is set to 3. if (wantbyte) writes the 57th byte, plus a byte
 of padding, len is decremented to 2 and totlen incremented to 58. The if
 (len > 0) writes another 2 bytes of padding and sets totlen to 60.
 
 At least that's my understanding of how things should work :)