[Libvirt-cim] [PATCH 5/8] RASD: Provider Support for Console RASDs

[Viktor Mihajlovski]

On 09/10/2013 11:11 PM, John Ferlan wrote:
[...]
>> +        CMSetProperty(inst, "TargetType",
>> +                      (CMPIValue *)cdev->target_type, CMPI_chars);
>> +
>
> Since libxkutil/parse_console_device() didn't do any error checking
> w/r/t a NULL return on get_attr_value() calls, we could get into a lot
> of trouble here, but again par for the course in libvirt-cim!
>
The CMPI interface allows you to explicitly set NULL string properties,
so  no trouble should arise from that at least (I have also checked
the code in sfcb and Pegasus to be 100% sure).
>> +        switch (cdev->source_type) {
>> +        case CIM_CHARDEV_SOURCE_TYPE_PTY:
>> +                CMSetProperty(inst, "SourcePath",
>> +                              (CMPIValue *)cdev->source_dev.pty.path,
>> +                              CMPI_chars);
>> +                break;
>> +        case CIM_CHARDEV_SOURCE_TYPE_DEV:
>> +                CMSetProperty(inst, "SourcePath",
>> +                              (CMPIValue *)cdev->source_dev.dev.path,
>> +                              CMPI_chars);
>> +                break;
>> +        case CIM_CHARDEV_SOURCE_TYPE_FILE:
>> +                CMSetProperty(inst, "SourcePath",
>> +                              (CMPIValue *)cdev->source_dev.file.path,
>> +                              CMPI_chars);
>> +                break;
>> +        case CIM_CHARDEV_SOURCE_TYPE_PIPE:
>> +                CMSetProperty(inst, "SourcePath",
>> +                              (CMPIValue *)cdev->source_dev.pipe.path,
>> +                              CMPI_chars);
>> +                break;
>> +        case CIM_CHARDEV_SOURCE_TYPE_UNIXSOCK:
>> +                tmp = _build_console_url("file",
>> +                                         cdev->source_dev.unixsock.path, NULL);
>> +                if (STREQC(cdev->source_dev.unixsock.mode, "bind"))
>> +                        CMSetProperty(inst, "BindURL",
>> +                                      (CMPIValue *)tmp, CMPI_chars);
>> +
>> +                if (STREQC(cdev->source_dev.unixsock.mode, "connect"))
>> +                        CMSetProperty(inst, "ConnectURL",
>> +                                      (CMPIValue *)tmp, CMPI_chars);
>> +
>> +                free(tmp);
>> +                break;
>> +        case CIM_CHARDEV_SOURCE_TYPE_UDP:
>> +                tmp = _build_console_url("udp",
>> +                                         cdev->source_dev.udp.bind_host,
>> +                                         cdev->source_dev.udp.bind_service);
>
> Since 'tmp' can be NULL, could there be any negative repercussion in the
> following?
no, see above
>
>> +                CMSetProperty(inst, "BindURL",
>> +                              (CMPIValue *)tmp, CMPI_chars);
>> +                free(tmp);
>> +
>> +                tmp = _build_console_url("udp",
>> +                                         cdev->source_dev.udp.connect_host,
>> +                                         cdev->source_dev.udp.connect_service);
>
> Same here
>
>> +                CMSetProperty(inst, "ConnectURL", (CMPIValue *)tmp, CMPI_chars);
>> +                free(tmp);
>> +                break;
>> +        case CIM_CHARDEV_SOURCE_TYPE_TCP:
>> +                tmp = _build_console_url(cdev->source_dev.tcp.protocol,
>> +                                         cdev->source_dev.tcp.host,
>> +                                         cdev->source_dev.tcp.service);
>> +                if (STREQC(cdev->source_dev.tcp.mode, "bind"))
>
> Again
that is probably not good ... will fix in V2
>
>> +                        CMSetProperty(inst, "BindURL",
>> +                                      (CMPIValue *)tmp, CMPI_chars);
>> +
>> +                if (STREQC(cdev->source_dev.tcp.mode, "connect"))
>
> Again
yep


-- 

Mit freundlichen Grüßen/Kind Regards
    Viktor Mihajlovski

IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Martina Köderitz
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294