[Libvirt-cim] [PATCH] HS: returns results for wrong object path

[Jay Gagnon]

Heidi Eckhart wrote:
> # HG changeset patch
> # User Heidi Eckhart <heidieck at linux.vnet.ibm.com>
> # Date 1199797461 -3600
> # Node ID 555126035e912e9202ebcc93483ee936be5ae998
> # Parent  1bbea87ad37dcb95321f43aa5e17e6fa7ec1836d
> HS: returns results for wrong object path
>
> The client given reference was not checked for existance in both
> directions (from HostSystem to Service and reverse) and the call
> returned wrong results. Now NOT_FOUND is returned in case of a
> wrong object path.
>
> Examples:
> wbemcli ain -ac KVM_HostedService 'http://localhost/root/virt:KVM_ResourcePoolConfigurationService.CreationClassName="wrong",wrong="wrong"'
> wbemcli ain -ac KVM_HostedService 'http://localhost/root/virt:KVM_HostSystem.CreationClassName="KVM_HostSystem",Name="notthere"'
>
> Signed-off-by: Heidi Eckhart <heidieck at linux.vnet.ibm.com>
>
> diff -r 1bbea87ad37d -r 555126035e91 src/Virt_HostedService.c
> --- a/src/Virt_HostedService.c	Mon Jan 07 11:42:40 2008 -0800
> +++ b/src/Virt_HostedService.c	Tue Jan 08 14:04:21 2008 +0100
> @@ -37,6 +37,57 @@
>
> +
> +static CMPIStatus check_service_ref(const CMPIObjectPath *ref)
> +{      
> +        CMPIStatus s = {CMPI_RC_OK, NULL};
> +        CMPIInstance *inst;
> +        const char *prop;
> +        char* classname;
> +
> +        classname = class_base_name(CLASSNAME(ref));
> +
> +        if (STREQC(classname, "VirtualSystemManagementService")) {
> +                s = get_vsms(ref, &inst, _BROKER);
> +        } 
> +        else if (STREQC(classname, "ResourcePoolConfigurationService")) {
> +                s = rpcs_instance(ref, &inst, _BROKER);
> +        }
> +        else if (STREQC(classname, "VirtualSystemMigrationService")) {
> +                s = get_migration_service(ref, &inst, _BROKER);
> +        }
> +        if (s.rc != CMPI_RC_OK)
> +                return s;
>   
Sorry for being so nitpicky, but for the sake of consistency I think
this is a place where we use a "goto out;" type of statement as opposed
to having multiple returns.
> +        
> +        prop = cu_compare_ref(ref, inst);
> +        if (prop != NULL) {
> +                cu_statusf(_BROKER, &s,
> +                           CMPI_RC_ERR_NOT_FOUND,
> +                           "No such instance (%s)", prop);
> +        }
> +        
>   
Putting out right here should work fine.  Actually, this is more than a
nitpick I think, because if the above return happens we don't hit this
free() and leak classname.
> +        free(classname);
> +
> +        return s;
> +}
>   


-- 

-Jay