On 09/01/2010 03:41 PM, Matthias Bolte wrote:
2010/9/1 Eric Blake<eblake@redhat.com>:
* src/openvz/openvz_conf.c (openvzLocateConfFile): Alter signature. (openvzGetVPSUUID, openvzSetDefinedUUID) (openvzWriteVPSConfigParam, openvzReadVPSConfigParam) (openvzCopyDefaultConfig): Adjust callers. ---
Nuke a few more PATH_MAX stack allocations.
src/openvz/openvz_conf.c | 78 ++++++++++++++++++++++++++++----------------- 1 files changed, 48 insertions(+), 30 deletions(-)
@@ -861,12 +869,18 @@ openvzGetVPSUUID(int vpsid, char *uuidstr, size_t len) uuidbuf = strtok_r(NULL, "\n",&saveptr);
if (iden != NULL&& uuidbuf != NULL&& STREQ(iden, "#UUID:")) { - if (virStrcpy(uuidstr, uuidbuf, len) == NULL) - retval = -1; + if (virStrcpy(uuidstr, uuidbuf, len) == NULL) { + virReportOOMError(); + goto cleanup; + }
virStrcpy cannot fail because of OOM, it doesn't do an allocation.
/me slaps forehead. Of course not; but at the same time, silently returning -1 is wrong.
qemuReportError(VIR_ERR_INTERNAL_ERROR, _("Unix socket '%s' too big for destination"), unixfile);
Unrelated, but the example you copied from reads a bit awkwardly (maybe "Unix socket too small to contain '%s'" would have sounded better?), but that's a separate patch.
break; } } - close(fd); + retval = 0; +cleanup: + if (0<= fd)
if (fd>= 0) reads nicer here.
That's coreutils HACKING policy rearing it's head (where Jim always prefers < over >); I've adjusted accordingly.
ACK, with these two comments addressed.
Thanks for the review; I squashed in the diff below, then pushed. That leaves 2, 3, and 5 of the series still awaiting review. diff --git i/src/openvz/openvz_conf.c w/src/openvz/openvz_conf.c index f00f2f4..ec11bbc 100644 --- i/src/openvz/openvz_conf.c +++ w/src/openvz/openvz_conf.c @@ -870,7 +870,8 @@ openvzGetVPSUUID(int vpsid, char *uuidstr, size_t len) if (iden != NULL && uuidbuf != NULL && STREQ(iden, "#UUID:")) { if (virStrcpy(uuidstr, uuidbuf, len) == NULL) { - virReportOOMError(); + openvzError(VIR_ERR_INTERNAL_ERROR, + _("invalid uuid %s"), uuidbuf); goto cleanup; } break; @@ -878,7 +879,7 @@ openvzGetVPSUUID(int vpsid, char *uuidstr, size_t len) } retval = 0; cleanup: - if (0 <= fd) + if (fd >= 0) close(fd); VIR_FREE(conf_file); -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org