2009/10/16 Cole Robinson <crobinso@xxxxxxxxxx>:
> These will be used by the test driver, so move them to a shareable space.
>
> Signed-off-by: Cole Robinson <crobinso@xxxxxxxxxx>
> ---
> src/conf/node_device_conf.c | 89 +++++++++++++++++++++++++++
> src/conf/node_device_conf.h | 11 +++
> src/libvirt_private.syms | 2 +
> src/node_device/node_device_driver.c | 112 ++++------------------------------
> 4 files changed, 115 insertions(+), 99 deletions(-)
>
> 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.
Matthias
--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list