[Libvir] [PATCH]OpenVZ Enhancements - Cleaned up
Hi, Attached are the OpenVZ driver enhancements. Cleanups have been done according to Daniel .V's suggestions. Please see my previous mail for summary. Cheers, Shuveb Hussain http://www.binarykarma.com
On Mon, Sep 03, 2007 at 08:01:00AM -0400, Shuveb Hussain wrote:
Hi,
Attached are the OpenVZ driver enhancements. Cleanups have been done according to Daniel .V's suggestions. Please see my previous mail for summary.
Okay, that looks better, thanks a lot ! I will apply them but there is a few things I still have issues with, I will try to fix them before commiting the patches: - in general using NULL to test anything but a pointer is not good code, thing like if(uuidstr[0] == (int)NULL) { is not okay. If you want to test a char for 0 use 0 not NULL. - I will have to check better but trying to detect an uninitialized UUID just by checking the first byte sounds rather wrong to me, but maybe I missed something. - some really bizare changes of an xmlDoc name field: + if (!(xml->name = calloc(1, PATH_MAX))) { + openvzLog(OPENVZ_ERR, "Error in allocating memory to store xml URI"); + xmlFreeDoc(xml); + return NULL; + } + if (displayName) + strncpy(xml->name, displayName, PATH_MAX - 1); + else + strncpy(xml->name, "domain.xml", PATH_MAX - 1); that's bad because: + it will probably leak the previous value of xml->name + xml->name should be allocated using xmlMalloc + why allocate MAX_PATH when you can allocate just the size so I will cleanup that part, still I don't undertsand what you tried to do there. This should hit CVS later today when I cleaned up those things and possibly a couple of warnings, thanks again ! Daniel -- Red Hat Virtualization group http://redhat.com/virtualization/ Daniel Veillard | virtualization library http://libvirt.org/ veillard@redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/
On Mon, Sep 03, 2007 at 08:39:27AM -0400, Daniel Veillard wrote:
On Mon, Sep 03, 2007 at 08:01:00AM -0400, Shuveb Hussain wrote:
Hi,
Attached are the OpenVZ driver enhancements. Cleanups have been done according to Daniel .V's suggestions. Please see my previous mail for summary.
Okay, that looks better, thanks a lot ! I will apply them but there is a few things I still have issues with, I will try to fix them before commiting the patches:
Okay I commited to CVS earlier today
- some really bizare changes of an xmlDoc name field: + if (!(xml->name = calloc(1, PATH_MAX))) { + openvzLog(OPENVZ_ERR, "Error in allocating memory to store xml URI"); + xmlFreeDoc(xml); + return NULL; + } + if (displayName) + strncpy(xml->name, displayName, PATH_MAX - 1); + else + strncpy(xml->name, "domain.xml", PATH_MAX - 1); that's bad because: + it will probably leak the previous value of xml->name + xml->name should be allocated using xmlMalloc + why allocate MAX_PATH when you can allocate just the size so I will cleanup that part, still I don't undertsand what you tried to do there.
I removed that part. The URI information passed to the xmlDocRead earlier should result in the same xml->URI being set, so I don't see the point of that duplication, dropped for now.
This should hit CVS later today when I cleaned up those things and possibly a couple of warnings,
I am also applying that second patch to clean thing up, this removes most warnings at compile time, fix a serious deallocation problem on error when listing the domains, and change strtoI to indicate it does not change the input string. There is still 3 serious warnings in openvz_conf.c around line 700, where the return value of write is not checked and as I indicated previously we should use a wrapper function handling interrupted calls at least. But that can be done later, thanks, Daniel -- Red Hat Virtualization group http://redhat.com/virtualization/ Daniel Veillard | virtualization library http://libvirt.org/ veillard@redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/
participants (2)
-
Daniel Veillard -
Shuveb Hussain