Several cleanups and fixes to mxge:

- Remove vestigial null pointer tests after malloc(..., M_WAITOK).

- Remove vestigal qualhack union

- Use strlcpy() instead of the error-prone strncpy() when parsing
  EEPROM and copying strings

- Check the MAC address in the EEPROM strings more strictly.

- Expand the macro MXGE_NEXT_STRING() at its only user. Due to a typo,
  the macro was very confusing.

- Remove unnecessary buffer limit check.  The buffer is double-NUL
  terminated per construction.

PR:		kern/176369
Submitted by:	Christoph Mallon <christoph.mallon gmx.de>

1 files changed, 25 insertions, 46 deletions

diff --git a/sys/dev/mxge/if_mxge.c b/sys/dev/mxge/if_mxge.c
index 245c139..68624ad 100644
--- a/sys/dev/mxge/if_mxge.c
+++ b/sys/dev/mxge/if_mxge.c
@@ -288,42 +288,43 @@ mxge_dma_free(mxge_dma_t *dma)
 static int
 mxge_parse_strings(mxge_softc_t *sc)
 {
-#define MXGE_NEXT_STRING(p) while(ptr < limit && *ptr++)
-
-	char *ptr, *limit;
+	char *ptr;
 	int i, found_mac, found_sn2;
+	char *endptr;
 
 	ptr = sc->eeprom_strings;
-	limit = sc->eeprom_strings + MXGE_EEPROM_STRINGS_SIZE;
 	found_mac = 0;
 	found_sn2 = 0;
-	while (ptr < limit && *ptr != '\0') {
-		if (memcmp(ptr, "MAC=", 4) == 0) {
-			ptr += 1;
-			sc->mac_addr_string = ptr;
-			for (i = 0; i < 6; i++) {
-				ptr += 3;
-				if ((ptr + 2) > limit)
+	while (*ptr != '\0') {
+		if (strncmp(ptr, "MAC=", 4) == 0) {
+			ptr += 4;
+			for (i = 0;;) {
+				sc->mac_addr[i] = strtoul(ptr, &endptr, 16);
+				if (endptr - ptr != 2)
+					goto abort;
+				ptr = endptr;
+				if (++i == 6)
+					break;
+				if (*ptr++ != ':')
 					goto abort;
-				sc->mac_addr[i] = strtoul(ptr, NULL, 16);
-				found_mac = 1;
 			}
-		} else if (memcmp(ptr, "PC=", 3) == 0) {
+			found_mac = 1;
+		} else if (strncmp(ptr, "PC=", 3) == 0) {
 			ptr += 3;
-			strncpy(sc->product_code_string, ptr,
-				sizeof (sc->product_code_string) - 1);
-		} else if (!found_sn2 && (memcmp(ptr, "SN=", 3) == 0)) {
+			strlcpy(sc->product_code_string, ptr,
+			    sizeof(sc->product_code_string));
+		} else if (!found_sn2 && (strncmp(ptr, "SN=", 3) == 0)) {
 			ptr += 3;
-			strncpy(sc->serial_number_string, ptr,
-				sizeof (sc->serial_number_string) - 1);
-		} else if (memcmp(ptr, "SN2=", 4) == 0) {
+			strlcpy(sc->serial_number_string, ptr,
+			    sizeof(sc->serial_number_string));
+		} else if (strncmp(ptr, "SN2=", 4) == 0) {
 			/* SN2 takes precedence over SN */
 			ptr += 4;
 			found_sn2 = 1;
-			strncpy(sc->serial_number_string, ptr,
-				sizeof (sc->serial_number_string) - 1);
+			strlcpy(sc->serial_number_string, ptr,
+			    sizeof(sc->serial_number_string));
 		}
-		MXGE_NEXT_STRING(ptr);
+		while (*ptr++ != '\0') {}
 	}
 
 	if (found_mac)
@@ -649,12 +650,6 @@ abort:
 	return (mxge_load_firmware(sc, 0));
 }
 
-union qualhack
-{
-        const char *ro_char;
-        char *rw_char;
-};
-
 static int
 mxge_validate_firmware(mxge_softc_t *sc, const mcp_gen_header_t *hdr)
 {
@@ -667,7 +662,7 @@ mxge_validate_firmware(mxge_softc_t *sc, const mcp_gen_header_t *hdr)
 	}
 
 	/* save firmware version for sysctl */
-	strncpy(sc->fw_version, hdr->version, sizeof (sc->fw_version));
+	strlcpy(sc->fw_version, hdr->version, sizeof(sc->fw_version));
 	if (mxge_verbose)
 		device_printf(sc->dev, "firmware id: %s\n", hdr->version);
 
@@ -3325,8 +3320,6 @@ mxge_alloc_slice_rings(struct mxge_slice_state *ss, int rx_ring_entries,
 	size_t bytes;
 	int err, i;
 
-	err = ENOMEM;
-
 	/* allocate per-slice receive resources */
 
 	ss->rx_small.mask = ss->rx_big.mask = rx_ring_entries - 1;
@@ -3335,24 +3328,16 @@ mxge_alloc_slice_rings(struct mxge_slice_state *ss, int rx_ring_entries,
 	/* allocate the rx shadow rings */
 	bytes = rx_ring_entries * sizeof (*ss->rx_small.shadow);
 	ss->rx_small.shadow = malloc(bytes, M_DEVBUF, M_ZERO|M_WAITOK);
-	if (ss->rx_small.shadow == NULL)
-		return err;
 
 	bytes = rx_ring_entries * sizeof (*ss->rx_big.shadow);
 	ss->rx_big.shadow = malloc(bytes, M_DEVBUF, M_ZERO|M_WAITOK);
-	if (ss->rx_big.shadow == NULL)
-		return err;
 
 	/* allocate the rx host info rings */
 	bytes = rx_ring_entries * sizeof (*ss->rx_small.info);
 	ss->rx_small.info = malloc(bytes, M_DEVBUF, M_ZERO|M_WAITOK);
-	if (ss->rx_small.info == NULL)
-		return err;
 
 	bytes = rx_ring_entries * sizeof (*ss->rx_big.info);
 	ss->rx_big.info = malloc(bytes, M_DEVBUF, M_ZERO|M_WAITOK);
-	if (ss->rx_big.info == NULL)
-		return err;
 
 	/* allocate the rx busdma resources */
 	err = bus_dma_tag_create(sc->parent_dmat,	/* parent */
@@ -3449,8 +3434,6 @@ mxge_alloc_slice_rings(struct mxge_slice_state *ss, int rx_ring_entries,
 	bytes = 8 + 
 		sizeof (*ss->tx.req_list) * (ss->tx.max_desc + 4);
 	ss->tx.req_bytes = malloc(bytes, M_DEVBUF, M_WAITOK);
-	if (ss->tx.req_bytes == NULL)
-		return err;
 	/* ensure req_list entries are aligned to 8 bytes */
 	ss->tx.req_list = (mcp_kreq_ether_send_t *)
 		((unsigned long)(ss->tx.req_bytes + 7) & ~7UL);
@@ -3459,14 +3442,10 @@ mxge_alloc_slice_rings(struct mxge_slice_state *ss, int rx_ring_entries,
 	bytes = sizeof (*ss->tx.seg_list) * ss->tx.max_desc;
 	ss->tx.seg_list = (bus_dma_segment_t *) 
 		malloc(bytes, M_DEVBUF, M_WAITOK);
-	if (ss->tx.seg_list == NULL)
-		return err;
 
 	/* allocate the tx host info ring */
 	bytes = tx_ring_entries * sizeof (*ss->tx.info);
 	ss->tx.info = malloc(bytes, M_DEVBUF, M_ZERO|M_WAITOK);
-	if (ss->tx.info == NULL)
-		return err;
 	
 	/* allocate the tx busdma resources */
 	err = bus_dma_tag_create(sc->parent_dmat,	/* parent */