[libvirt] [PATCH 7/7] openvz: use virAsprintf to avoid large stacks

[Eric Blake]

On 09/01/2010 03:41 PM, Matthias Bolte wrote:
> 2010/9/1 Eric Blake<eblake at 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 at redhat.com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org