Re: [libvirt] [PATCH 7/7] openvz: use virAsprintf to avoid large stacks
On 09/01/2010 03:41 PM, Matthias Bolte wrote:
2010/9/1 Eric Blake<eblake(a)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(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org