On 09/10/2013 10:38 PM, John Ferlan wrote: [...]
+ + CU_DEBUG("console device type ID = %d", cdev->source_type); + free(source_type_str);
This is free()'d again in err:, so we get a Coverity complaint about a double free. You need a "source_type_str = NULL;" here. oops ... will fix by moving to the end
+ + for (child = node->children; child != NULL; child = child->next) { + if (XSTREQ(child->name, "target")) { + cdev->target_type = get_attr_value(child, "type"); + CU_DEBUG("Console device target type = '%s'", + cdev->target_type);
If get_attr_value() returns NULL in cdev->target_type, then your CU_DEBUG() isn't going to be happy. actually, I expect CU_DEBUG to handle that gracefully... as it does in other places (see parse_graphics_device, vnc port determination)
[...]
+ } + free(udp_source_mode);
This is free()'d again in err:, so we get a Coverity complaint about a double free. You'll need a udp_source_mode = NULL; here. yep
+ break; + case CIM_CHARDEV_SOURCE_TYPE_TCP: + cdev->source_dev.tcp.mode = + get_attr_value(child, "mode"); + cdev->source_dev.tcp.host = + get_attr_value(child, "host"); + cdev->source_dev.tcp.service = + get_attr_value(child, "service"); + break; + + default: + /* Nothing to do for : + CIM_CHARDEV_SOURCE_TYPE_STDIO + CIM_CHARDEV_SOURCE_TYPE_NULL + CIM_CHARDEV_SOURCE_TYPE_VC + CIM_CHARDEV_SOURCE_TYPE_SPICEVMC + */ + break; + } + }
Not a whole lot of NULL error checking in any of the get_attr_value() calls, but that seems to be par for the course in the libvirt-cim code. at some point in time we should consider a 'spring clean'
The rest seemed fine to me. I can squash in the above listed changes before pushing unless you really want to make a v2. I'll make a v2, as I saw some other nits, like [...]
+ if (-1 == asprintf(&vdev->id, "charconsole:%s", target_port_ID)) { (left hand side constant in comparison) which escaped my attention before ... + CU_DEBUG("Failed to create charconsole id string"); + goto err; + }
-- 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