PERFORCE change 112957 for review

Hans Petter Selasky hselasky at FreeBSD.org
Mon Jan 15 22:55:24 UTC 2007


http://perforce.freebsd.org/chv.cgi?CH=112957

Change 112957 by hselasky at hselasky_mini_itx on 2007/01/15 22:54:41

	Fix problems and memory leaks regarding calls to "device_get_children()".

Affected files ...

.. //depot/projects/usb/src/sys/amd64/pci/pci_bus.c#2 edit
.. //depot/projects/usb/src/sys/compat/linsysfs/linsysfs.c#2 edit
.. //depot/projects/usb/src/sys/compat/ndis/subr_ntoskrnl.c#2 edit
.. //depot/projects/usb/src/sys/dev/acpica/acpi.c#3 edit
.. //depot/projects/usb/src/sys/dev/acpica/acpi_pci.c#2 edit
.. //depot/projects/usb/src/sys/dev/ata/ata-all.c#3 edit
.. //depot/projects/usb/src/sys/dev/ata/ata-cbus.c#2 edit
.. //depot/projects/usb/src/sys/dev/ata/ata-chipset.c#3 edit
.. //depot/projects/usb/src/sys/dev/ata/ata-disk.c#3 edit
.. //depot/projects/usb/src/sys/dev/ata/ata-pci.c#2 edit
.. //depot/projects/usb/src/sys/dev/ata/atapi-cam.c#2 edit
.. //depot/projects/usb/src/sys/dev/cardbus/cardbus.c#3 edit
.. //depot/projects/usb/src/sys/dev/cardbus/cardbus_device.c#2 edit
.. //depot/projects/usb/src/sys/dev/esp/esp_sbus.c#2 edit
.. //depot/projects/usb/src/sys/dev/hme/if_hme_pci.c#2 edit
.. //depot/projects/usb/src/sys/dev/mii/mlphy.c#3 edit
.. //depot/projects/usb/src/sys/dev/mii/tlphy.c#3 edit
.. //depot/projects/usb/src/sys/dev/pccard/pccard_device.c#2 edit
.. //depot/projects/usb/src/sys/dev/pccbb/pccbb.c#3 edit
.. //depot/projects/usb/src/sys/dev/pci/pci.c#3 edit
.. //depot/projects/usb/src/sys/dev/ppbus/ppbconf.c#2 edit
.. //depot/projects/usb/src/sys/dev/ppc/ppc.c#2 edit
.. //depot/projects/usb/src/sys/dev/sound/pci/csa.c#2 edit
.. //depot/projects/usb/src/sys/dev/sound/pci/emu10kx.c#2 edit
.. //depot/projects/usb/src/sys/dev/spibus/spibus.c#2 edit
.. //depot/projects/usb/src/sys/i386/pci/pci_bus.c#2 edit
.. //depot/projects/usb/src/sys/isa/isa_common.c#3 edit
.. //depot/projects/usb/src/sys/kern/kern_cpu.c#2 edit
.. //depot/projects/usb/src/sys/kern/subr_bus.c#3 edit
.. //depot/projects/usb/src/sys/pci/agp.c#3 edit
.. //depot/projects/usb/src/sys/pci/agp_i810.c#3 edit
.. //depot/projects/usb/src/sys/pci/if_sis.c#3 edit
.. //depot/projects/usb/src/sys/sys/bus.h#3 edit

Differences ...

==== //depot/projects/usb/src/sys/amd64/pci/pci_bus.c#2 (text+ko) ====


==== //depot/projects/usb/src/sys/compat/linsysfs/linsysfs.c#2 (text) ====

@@ -214,10 +214,12 @@
 		}
 	}
 
-	device_get_children(dev, &children, &nchildren);
-	for (i = 0; i < nchildren; i++) {
+	if (!device_get_children(dev, &children, &nchildren)) {
+	    for (i = 0; i < nchildren; i++) {
 		if (children[i])
 			linsysfs_run_bus(children[i], dir, scsi, new_path, prefix);
+	    }
+	    free(children, M_TEMP);
 	}
 	if (new_path != path)
 		free(new_path, M_TEMP);

==== //depot/projects/usb/src/sys/compat/ndis/subr_ntoskrnl.c#2 (text+ko) ====

@@ -2662,22 +2662,17 @@
 	 * level of recursion to inspect them.
 	 */
 
-	device_get_children(dev, &children, &childcnt);
-
-	for (i = 0; i < childcnt; i++) {
+	if (!device_get_children(dev, &children, &childcnt)) {
+	    for (i = 0; i < childcnt; i++) {
 		matching_dev = ntoskrnl_finddev(children[i], paddr, res);
 		if (matching_dev != NULL) {
 			free(children, M_TEMP);
 			return(matching_dev);
 		}
+	    }
+	    free(children, M_TEMP);
 	}
 
-	
-	/* Won't somebody please think of the children! */
-
-	if (children != NULL)
-		free(children, M_TEMP);
-
 	return(NULL);
 }
 

==== //depot/projects/usb/src/sys/dev/acpica/acpi.c#3 (text+ko) ====

@@ -625,8 +625,8 @@
      * device has an _SxD method for the next sleep state, use that power
      * state instead.
      */
-    device_get_children(dev, &devlist, &numdevs);
-    for (i = 0; i < numdevs; i++) {
+    if (!device_get_children(dev, &devlist, &numdevs)) {
+      for (i = 0; i < numdevs; i++) {
 	/* If the device is not attached, we've powered it down elsewhere. */
 	child = devlist[i];
 	if (!device_is_attached(child))
@@ -641,8 +641,9 @@
 	    child, &pstate);
 	if ((error == 0 || error == ESRCH) && acpi_do_powerstate)
 	    pci_set_powerstate(child, pstate);
+      }
+      free(devlist, M_TEMP);
     }
-    free(devlist, M_TEMP);
     error = 0;
 
     return (error);
@@ -661,16 +662,17 @@
      * Put all devices in D0 before resuming them.  Call _S0D on each one
      * since some systems expect this.
      */
-    device_get_children(dev, &devlist, &numdevs);
-    for (i = 0; i < numdevs; i++) {
+    if (!device_get_children(dev, &devlist, &numdevs)) {
+      for (i = 0; i < numdevs; i++) {
 	child = devlist[i];
 	handle = acpi_get_handle(child);
 	if (handle)
 	    AcpiEvaluateObject(handle, "_S0D", NULL, NULL);
 	if (device_is_attached(child) && acpi_do_powerstate)
 	    pci_set_powerstate(child, PCI_POWERSTATE_D0);
+      }
+      free(devlist, M_TEMP);
     }
-    free(devlist, M_TEMP);
 
     return (bus_generic_resume(dev));
 }
@@ -761,16 +763,17 @@
     int i, numdevs;
 
     DEVICE_IDENTIFY(driver, dev);
-    device_get_children(dev, &devlist, &numdevs);
-    for (i = 0; i < numdevs; i++) {
+    if (!device_get_children(dev, &devlist, &numdevs)) {
+      for (i = 0; i < numdevs; i++) {
 	child = devlist[i];
 	if (device_get_state(child) == DS_NOTPRESENT) {
 	    /* pci_set_powerstate(child, PCI_POWERSTATE_D0); */
 	    if (device_probe_and_attach(child) != 0)
 		; /* pci_set_powerstate(child, PCI_POWERSTATE_D3); */
 	}
+      }
+      free(devlist, M_TEMP);
     }
-    free(devlist, M_TEMP);
 }
 
 /* Location hint for devctl(8) */
@@ -2404,7 +2407,7 @@
 
     error = device_get_children(dev, &devlist, &numdevs);
     if (error != 0 || numdevs == 0) {
-	if (numdevs == 0)
+	if (error == 0)
 	    free(devlist, M_TEMP);
 	return (error);
     }

==== //depot/projects/usb/src/sys/dev/acpica/acpi_pci.c#2 (text+ko) ====


==== //depot/projects/usb/src/sys/dev/ata/ata-all.c#3 (text+ko) ====


==== //depot/projects/usb/src/sys/dev/ata/ata-cbus.c#2 (text+ko) ====

@@ -261,12 +261,13 @@
     int count, i;
 
     /* find channel number on this controller */
-    device_get_children(device_get_parent(dev), &children, &count);
-    for (i = 0; i < count; i++) {
+    if (!device_get_children(device_get_parent(dev), &children, &count)) {
+      for (i = 0; i < count; i++) {
 	if (children[i] == dev) 
 	    ch->unit = i;
+      }
+      free(children, M_TEMP);
     }
-    free(children, M_TEMP);
 
     /* setup the resource vectors */
     for (i = ATA_DATA; i <= ATA_COMMAND; i ++) {

==== //depot/projects/usb/src/sys/dev/ata/ata-chipset.c#3 (text+ko) ====


==== //depot/projects/usb/src/sys/dev/ata/ata-disk.c#3 (text+ko) ====


==== //depot/projects/usb/src/sys/dev/ata/ata-pci.c#2 (text+ko) ====

@@ -539,12 +539,13 @@
     bzero(ch, sizeof(struct ata_channel));
 
     /* find channel number on this controller */
-    device_get_children(device_get_parent(dev), &children, &count);
-    for (i = 0; i < count; i++) {
+    if (!device_get_children(device_get_parent(dev), &children, &count)) {
+      for (i = 0; i < count; i++) {
 	if (children[i] == dev)
 	    ch->unit = i;
+      }
+      free(children, M_TEMP);
     }
-    free(children, M_TEMP);
 
     sprintf(buffer, "ATA channel %d", ch->unit);
     device_set_desc_copy(dev, buffer);

==== //depot/projects/usb/src/sys/dev/ata/atapi-cam.c#2 (text+ko) ====


==== //depot/projects/usb/src/sys/dev/cardbus/cardbus.c#3 (text+ko) ====

@@ -217,7 +217,8 @@
 	int tmp;
 	int err = 0;
 
-	device_get_children(cbdev, &devlist, &numdevs);
+	if (device_get_children(cbdev, &devlist, &numdevs))
+		return ENOENT;
 
 	if (numdevs == 0) {
 		free(devlist, M_TEMP);
@@ -251,7 +252,8 @@
 	struct cardbus_devinfo *dinfo;
 
 	DEVICE_IDENTIFY(driver, cbdev);
-	device_get_children(cbdev, &devlist, &numdevs);
+	if (device_get_children(cbdev, &devlist, &numdevs))
+		return;
 	/*
 	 * If there are no drivers attached, but there are children,
 	 * then power the card up.

==== //depot/projects/usb/src/sys/dev/cardbus/cardbus_device.c#2 (text+ko) ====


==== //depot/projects/usb/src/sys/dev/esp/esp_sbus.c#2 (text+ko) ====


==== //depot/projects/usb/src/sys/dev/hme/if_hme_pci.c#2 (text+ko) ====


==== //depot/projects/usb/src/sys/dev/mii/mlphy.c#3 (text+ko) ====

@@ -195,14 +195,15 @@
 	 * See if there's another PHY on this bus with us.
 	 * If so, we may need it for 10Mbps modes.
 	 */
-	device_get_children(msc->ml_mii.mii_dev, &devlist, &devs);
-	for (i = 0; i < devs; i++) {
+	if (!device_get_children(msc->ml_mii.mii_dev, &devlist, &devs)) {
+	    for (i = 0; i < devs; i++) {
 		if (strcmp(device_get_name(devlist[i]), "mlphy")) {
 			other = device_get_softc(devlist[i]);
 			break;
 		}
+	    }
+	    free(devlist, M_TEMP);
 	}
-	free(devlist, M_TEMP);
 
 	switch (cmd) {
 	case MII_POLLSTAT:
@@ -404,14 +405,15 @@
 	int			devs, i;
 
 	/* See if there's another PHY on the bus with us. */
-	device_get_children(msc->ml_mii.mii_dev, &devlist, &devs);
-	for (i = 0; i < devs; i++) {
+	if (!device_get_children(msc->ml_mii.mii_dev, &devlist, &devs)) {
+	    for (i = 0; i < devs; i++) {
 		if (strcmp(device_get_name(devlist[i]), "mlphy")) {
 			other = device_get_softc(devlist[i]);
 			break;
 		}
+	    }
+	    free(devlist, M_TEMP);
 	}
-	free(devlist, M_TEMP);
 
 	if (other == NULL)
 		return;

==== //depot/projects/usb/src/sys/dev/mii/tlphy.c#3 (text+ko) ====

@@ -168,15 +168,16 @@
 		device_t		*devlist;
 		int			devs, i;
 
-		device_get_children(sc->sc_mii.mii_dev, &devlist, &devs);
-		for (i = 0; i < devs; i++) {
+		if (!device_get_children(sc->sc_mii.mii_dev, &devlist, &devs)) {
+		    for (i = 0; i < devs; i++) {
 			if (strcmp(device_get_name(devlist[i]), "tlphy")) {
 				other = device_get_softc(devlist[i]);
 				capmask &= ~other->mii_capabilities;
 				break;
 			}
+		    }
+		    free(devlist, M_TEMP);
 		}
-		free(devlist, M_TEMP);
 	}
 
 	mii->mii_instance++;

==== //depot/projects/usb/src/sys/dev/pccard/pccard_device.c#2 (text+ko) ====


==== //depot/projects/usb/src/sys/dev/pccbb/pccbb.c#3 (text+ko) ====

@@ -302,10 +302,11 @@
 	 * for the kldload/unload case to work.  If we failed to do that, then
 	 * we'd get duplicate devices when cbb.ko was reloaded.
 	 */
-	device_get_children(brdev, &devlist, &numdevs);
-	for (tmp = 0; tmp < numdevs; tmp++)
-		device_delete_child(brdev, devlist[tmp]);
-	free(devlist, M_TEMP);
+	if (!device_get_children(brdev, &devlist, &numdevs)) {
+		for (tmp = 0; tmp < numdevs; tmp++)
+			device_delete_child(brdev, devlist[tmp]);
+		free(devlist, M_TEMP);
+	}
 
 	/* Turn off the interrupts */
 	cbb_set(sc, CBB_SOCKET_MASK, 0);
@@ -413,14 +414,15 @@
 	int wake = 0;
 
 	DEVICE_IDENTIFY(driver, brdev);
-	device_get_children(brdev, &devlist, &numdevs);
-	for (tmp = 0; tmp < numdevs; tmp++) {
+	if (!device_get_children(brdev, &devlist, &numdevs)) {
+	    for (tmp = 0; tmp < numdevs; tmp++) {
 		dev = devlist[tmp];
 		if (device_get_state(dev) == DS_NOTPRESENT &&
 		    device_probe_and_attach(dev) == 0)
 			wake++;
+	    }
+	    free(devlist, M_TEMP);
 	}
-	free(devlist, M_TEMP);
 
 	if (wake > 0) {
 		mtx_lock(&sc->mtx);

==== //depot/projects/usb/src/sys/dev/pci/pci.c#3 (text+ko) ====

@@ -1193,7 +1193,8 @@
 	acpi_dev = NULL;
 	if (pci_do_power_resume)
 		acpi_dev = devclass_get_device(devclass_find("acpi"), 0);
-	device_get_children(dev, &devlist, &numdevs);
+	if (device_get_children(dev, &devlist, &numdevs))
+		return ENOMEM;
 	for (i = 0; i < numdevs; i++) {
 		child = devlist[i];
 		dinfo = (struct pci_devinfo *) device_get_ivars(child);
@@ -1240,7 +1241,8 @@
 	acpi_dev = NULL;
 	if (pci_do_power_resume)
 		acpi_dev = devclass_get_device(devclass_find("acpi"), 0);
-	device_get_children(dev, &devlist, &numdevs);
+	if (device_get_children(dev, &devlist, &numdevs))
+		return ENOMEM;
 	for (i = 0; i < numdevs; i++) {
 		/*
 		 * Notify ACPI we're going to D0 but ignore the result.  If
@@ -1289,8 +1291,8 @@
 	if (bootverbose)
 		device_printf(dev, "driver added\n");
 	DEVICE_IDENTIFY(driver, dev);
-	device_get_children(dev, &devlist, &numdevs);
-	for (i = 0; i < numdevs; i++) {
+	if (!device_get_children(dev, &devlist, &numdevs)) {
+	    for (i = 0; i < numdevs; i++) {
 		child = devlist[i];
 		if (device_get_state(child) != DS_NOTPRESENT)
 			continue;
@@ -1302,8 +1304,9 @@
 		pci_cfg_restore(child, dinfo);
 		if (device_probe_and_attach(child) != 0)
 			pci_cfg_save(child, dinfo, 1);
+	    }
+	    free(devlist, M_TEMP);
 	}
-	free(devlist, M_TEMP);
 }
 
 int

==== //depot/projects/usb/src/sys/dev/ppbus/ppbconf.c#2 (text+ko) ====


==== //depot/projects/usb/src/sys/dev/ppc/ppc.c#2 (text+ko) ====


==== //depot/projects/usb/src/sys/dev/sound/pci/csa.c#2 (text+ko) ====

@@ -120,8 +120,8 @@
 
 	for (i = 0, busp = pci_devices; i < pci_count; i++, busp++) {
 		pci_childcount = 0;
-		device_get_children(*busp, &pci_children, &pci_childcount);
-		for (j = 0, childp = pci_children; j < pci_childcount; j++, childp++) {
+		if (!device_get_children(*busp, &pci_children, &pci_childcount)) {
+		    for (j = 0, childp = pci_children; j < pci_childcount; j++, childp++) {
 			if (pci_get_vendor(*childp) == 0x8086 && pci_get_device(*childp) == 0x7113) {
 				port = (pci_read_config(*childp, 0x41, 1) << 8) + 0x10;
 				/* XXX */
@@ -135,8 +135,9 @@
 				free(pci_children, M_TEMP);
 				return 0;
 			}
+		    }
+		    free(pci_children, M_TEMP);
 		}
-		free(pci_children, M_TEMP);
 	}
 
 	free(pci_devices, M_TEMP);

==== //depot/projects/usb/src/sys/dev/sound/pci/emu10kx.c#2 (text+ko) ====

@@ -2949,12 +2949,13 @@
 		r = device_delete_child(dev, sc->midi[1]);
 	if (r)
 		return (r);
-	(void)device_get_children(dev, &childlist, &devcount);
-	for (i = 0; i < devcount - 1; i++) {
+	if (!device_get_children(dev, &childlist, &devcount)) {
+	    for (i = 0; i < devcount - 1; i++) { /* XXX why is there "-1" here ? */
 		device_printf(dev, "removing stale child %d (unit %d)\n", i, device_get_unit(childlist[i]));
 		device_delete_child(dev, childlist[i]);
+	    }
+	    free(childlist, M_TEMP);
 	}
-	free(childlist, M_TEMP);
 
 	/* shutdown chip */
 	emu_uninit(sc);

==== //depot/projects/usb/src/sys/dev/spibus/spibus.c#2 (text+ko) ====


==== //depot/projects/usb/src/sys/i386/pci/pci_bus.c#2 (text+ko) ====


==== //depot/projects/usb/src/sys/isa/isa_common.c#3 (text+ko) ====


==== //depot/projects/usb/src/sys/kern/kern_cpu.c#2 (text+ko) ====


==== //depot/projects/usb/src/sys/kern/subr_bus.c#3 (text+ko) ====

@@ -1849,6 +1849,7 @@
 device_get_children(device_t dev, device_t **devlistp, int *devcountp)
 {
 	int count;
+	int n;
 	device_t child;
 	device_t *list;
 
@@ -1857,14 +1858,31 @@
 		count++;
 	}
 
-	list = malloc(count * sizeof(device_t), M_TEMP, M_NOWAIT|M_ZERO);
-	if (!list)
+	if (count == 0) {
+		/* avoid zero size allocation */
+		n = 1 * sizeof(device_t);
+	} else {
+		n = count * sizeof(device_t);
+	}
+
+	list = malloc(n, M_TEMP, M_NOWAIT|M_ZERO);
+	if (!list) {
+		*devlistp = NULL;
+		*devcountp = 0;
 		return (ENOMEM);
+	}
 
-	count = 0;
+	n = 0;
 	TAILQ_FOREACH(child, &dev->children, link) {
-		list[count] = child;
-		count++;
+		if (n < count) {
+			list[n] = child;
+			n++;
+		}
+	}
+
+	if (n != count) {
+		printf("device_get_children: Number of devices changed "
+		       "from %d to %d!\n", count, n);
 	}
 
 	*devlistp = list;

==== //depot/projects/usb/src/sys/pci/agp.c#3 (text+ko) ====

@@ -118,8 +118,8 @@
 		bus = devclass_get_device(pci, busnum);
 		if (!bus)
 			continue;
-		device_get_children(bus, &kids, &numkids);
-		for (i = 0; i < numkids; i++) {
+		if (!device_get_children(bus, &kids, &numkids)) {
+		    for (i = 0; i < numkids; i++) {
 			dev = kids[i];
 			if (pci_get_class(dev) == PCIC_DISPLAY
 			    && pci_get_subclass(dev) == PCIS_DISPLAY_VGA)
@@ -128,8 +128,9 @@
 					return dev;
 				}
 					
+		    }
+		    free(kids, M_TEMP);
 		}
-		free(kids, M_TEMP);
 	}
 
 	return 0;

==== //depot/projects/usb/src/sys/pci/agp_i810.c#3 (text+ko) ====


==== //depot/projects/usb/src/sys/pci/if_sis.c#3 (text+ko) ====

@@ -349,20 +349,22 @@
 
 	for (i = 0, busp = pci_devices; i < pci_count; i++, busp++) {
 		pci_childcount = 0;
-		device_get_children(*busp, &pci_children, &pci_childcount);
-		for (j = 0, childp = pci_children;
+		if (!device_get_children(*busp, &pci_children, &pci_childcount)) {
+		  for (j = 0, childp = pci_children;
 		    j < pci_childcount; j++, childp++) {
 			if (pci_get_vendor(*childp) == SIS_VENDORID &&
 			    pci_get_device(*childp) == 0x0008) {
 				child = *childp;
+				free(pci_children, M_TEMP);
 				goto done;
 			}
+		  }
+		  free(pci_children, M_TEMP);
 		}
 	}
 
 done:
 	free(pci_devices, M_TEMP);
-	free(pci_children, M_TEMP);
 	return(child);
 }
 

==== //depot/projects/usb/src/sys/sys/bus.h#3 (text+ko) ====



More information about the p4-projects mailing list