[PATCH 0 of 2] #4 - SDC: Fixed type of returned instance and error code
Patch 1: The returned RASD instances have been of the general classname ResourceAllocationSettingData instead of the type specific one, e.g. ProcResourceAllocationSettingData Patch 2: RASD to AllocationCapabilities should return NOT_SUPPORTED instead of OK Diff to #1: patch 1: fixed missing check for unknown resource type Diff to #2: - added helper function to RASD that translates the resource type into the classname (patch 1 of 3) - updated patch 2 (#1 before) to make use of the helper function - updated patch 3 (#2 before) to make use of RETURN_UNSUPPORTED macro Diff to #3: - patch 1 and 3 are already checked in and no longer part of this patch series - added patch to harden free_rasd_prop_list() function to handle NULL pointer - patch 2 (also patch 2 before): cleaned up error handling
# HG changeset patch # User Heidi Eckhart <heidieck@linux.vnet.ibm.com> # Date 1197881324 -3600 # Node ID 109fd77699732a1bc2969b93d9a799c446b3f4c0 # Parent 72b93a4339e1c13080d6a6d80a580a7742c0b851 SDC: harden free_rasd_prop_list to handle NULL pointer Signed-off-by: Heidi Eckhart <heidieck@linux.vnet.ibm.com> diff -r 72b93a4339e1 -r 109fd7769973 src/Virt_SettingsDefineCapabilities.c --- a/src/Virt_SettingsDefineCapabilities.c Fri Dec 14 10:37:01 2007 +0100 +++ b/src/Virt_SettingsDefineCapabilities.c Mon Dec 17 09:48:44 2007 +0100 @@ -89,17 +89,19 @@ static bool dup_rasd_prop_list(struct sd return true; } -static bool free_rasd_prop_list(struct sdc_rasd_prop *prop_list) +static void free_rasd_prop_list(struct sdc_rasd_prop *prop_list) { int i; + + if (!prop_list) + return; for (i = 0; prop_list[i].field != NULL; i++) { free(prop_list[i].field); free(prop_list[i].value); } - + free (prop_list); - return true; } static struct sdc_rasd_prop *mem_max(const CMPIObjectPath *ref,
# HG changeset patch # User Heidi Eckhart <heidieck@linux.vnet.ibm.com> # Date 1197881325 -3600 # Node ID c22a61c0852055c920f46ffe82ab5761c7240444 # Parent 109fd77699732a1bc2969b93d9a799c446b3f4c0 SDC: returned instances are not of type specific subclass Signed-off-by: Heidi Eckhart <heidieck@linux.vnet.ibm.com> diff -r 109fd7769973 -r c22a61c08520 src/Virt_SettingsDefineCapabilities.c --- a/src/Virt_SettingsDefineCapabilities.c Mon Dec 17 09:48:44 2007 +0100 +++ b/src/Virt_SettingsDefineCapabilities.c Mon Dec 17 09:48:45 2007 +0100 @@ -42,6 +42,7 @@ #include "Virt_SettingsDefineCapabilities.h" #include "Virt_DevicePool.h" +#include "Virt_RASD.h" const static CMPIBroker *_BROKER; @@ -657,9 +658,10 @@ static CMPIInstance *sdc_rasd_inst(const sdc_rasd_type type) { CMPIInstance *inst = NULL; - struct sdc_rasd_prop *prop_list; + struct sdc_rasd_prop *prop_list = NULL; int i; - char *inst_id; + char *inst_id = NULL; + const char *base = NULL; uint16_t resource_type; /* Defaults for the following are from CIM_SettingsDefineCapabilities.mof. */ @@ -700,15 +702,21 @@ static CMPIInstance *sdc_rasd_inst(const cu_statusf(broker, s, CMPI_RC_ERR_FAILED, "Unsupported sdc_rasd type"); - goto out; } if (s->rc != CMPI_RC_OK) goto out; + + if (rasd_classname_from_type(rasd->resource_type, &base) != CMPI_RC_OK) { + cu_statusf(broker, s, + CMPI_RC_ERR_FAILED, + "Resource type not known"); + goto out; + } inst = get_typed_instance(broker, CLASSNAME(ref), - "ResourceAllocationSettingData", + base, NAMESPACE(ref)); CMSetProperty(inst, "InstanceID", inst_id, CMPI_chars); @@ -725,10 +733,8 @@ static CMPIInstance *sdc_rasd_inst(const prop_list[i].value, prop_list[i].type); } - CU_DEBUG("freeing prop_list"); + out: free_rasd_prop_list(prop_list); - out: - CU_DEBUG("Returning inst"); return inst; }
HE> # HG changeset patch HE> # User Heidi Eckhart <heidieck@linux.vnet.ibm.com> HE> # Date 1197881325 -3600 HE> # Node ID c22a61c0852055c920f46ffe82ab5761c7240444 HE> # Parent 109fd77699732a1bc2969b93d9a799c446b3f4c0 HE> SDC: returned instances are not of type specific subclass This looks better, thanks a lot for making that change. However, I just noticed something else: HE> - char *inst_id; HE> + char *inst_id = NULL; inst_id should be a const char * as well. Since we're touching it here, I think we might as well fix that up. Sorry to force another iteration, but if you wouldn't mind tweaking that one thing, I promise I'll commit this set without further complaint :P Thanks Heidi! -- Dan Smith IBM Linux Technology Center Open Hypervisor Team email: danms@us.ibm.com
Dan Smith wrote:
HE> # HG changeset patch HE> # User Heidi Eckhart <heidieck@linux.vnet.ibm.com> HE> # Date 1197881325 -3600 HE> # Node ID c22a61c0852055c920f46ffe82ab5761c7240444 HE> # Parent 109fd77699732a1bc2969b93d9a799c446b3f4c0 HE> SDC: returned instances are not of type specific subclass
This looks better, thanks a lot for making that change. However, I just noticed something else:
HE> - char *inst_id; HE> + char *inst_id = NULL;
inst_id should be a const char * as well. Since we're touching it here, I think we might as well fix that up.
Good catch ! I will resend the patch set.
Sorry to force another iteration, but if you wouldn't mind tweaking that one thing, I promise I'll commit this set without further complaint :P
:) -- Regards Heidi Eckhart Software Engineer Linux Technology Center - Open Hypervisor heidieck@linux.vnet.ibm.com ************************************************** IBM Deutschland Entwicklung GmbH Vorsitzender des Aufsichtsrats: Martin Jetter Geschaeftsfuehrung: Herbert Kircher Sitz der Gesellschaft: Boeblingen Registergericht: Amtsgericht Stuttgart, HRB 243294
participants (2)
-
Dan Smith -
Heidi Eckhart