On 10/17/2009 08:58 AM, Matthias Bolte wrote:
diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c index f09f814..77f7be3 100644 --- a/src/conf/node_device_conf.c +++ b/src/conf/node_device_conf.c @@ -1215,6 +1215,95 @@ virNodeDeviceDefParseFile(virConnectPtr conn, Â Â return virNodeDeviceDefParse(conn, NULL, filename, create); Â }
+/* + * Return fc_host dev's WWNN and WWPN + */ +int +virNodeDeviceGetWWNs(virConnectPtr conn, + Â Â Â Â Â Â Â Â Â Â virNodeDeviceDefPtr def, + Â Â Â Â Â Â Â Â Â Â char **wwnn, + Â Â Â Â Â Â Â Â Â Â char **wwpn) +{ + Â Â virNodeDevCapsDefPtr cap = NULL; + Â Â int ret = 0; + + Â Â cap = def->caps; + Â Â while (cap != NULL) { + Â Â Â Â if (cap->type == VIR_NODE_DEV_CAP_SCSI_HOST && + Â Â Â Â Â Â cap->data.scsi_host.flags & VIR_NODE_DEV_CAP_FLAG_HBA_FC_HOST) { + Â Â Â Â Â Â *wwnn = strdup(cap->data.scsi_host.wwnn); + Â Â Â Â Â Â *wwpn = strdup(cap->data.scsi_host.wwpn); + Â Â Â Â Â Â break; + Â Â Â Â } + + Â Â Â Â cap = cap->next; + Â Â } + + Â Â if (cap == NULL) { + Â Â Â Â virNodeDeviceReportError(conn, VIR_ERR_NO_SUPPORT, + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â "%s", _("Device is not a fibre channel HBA")); + Â Â Â Â ret = -1; + Â Â } + + Â Â if (*wwnn == NULL || *wwpn == NULL) {
I know you have just moved this function to another file, but here seems to be a logical error that may result in a false-positive OOM error.
If the while loop is left without finding the wanted device (cap == NULL), then *wwnn and *wwpn have not been set inside this function and point to their initial value (probably NULL). Because they have not been set inside this function (in the cap == NULL case), they should not be tested for NULL to detect an OOM condition.
To fix this
if (*wwnn == NULL || *wwpn == NULL) {
could be changed to
else if (*wwnn == NULL || *wwpn == NULL) {
so the OOM check is only done if the wanted device has been found (cap != NULL).
+ Â Â Â Â /* Free the other one, if allocated... */ + Â Â Â Â VIR_FREE(wwnn); + Â Â Â Â VIR_FREE(wwpn); + Â Â Â Â ret = -1; + Â Â Â Â virReportOOMError(conn); + Â Â } + + Â Â return ret; +} +
The OOM detection should be fixed by an additional patch, so ACK to this one.
Good catch. I agree a follow up patch is the way to go. Thanks, Cole