[libvirt] [PATCH 3/5] virstring: Introduce virVasprintfNOOM and make virVasprintf report OOM

[Michal Privoznik]

On 05.04.2013 12:45, Daniel P. Berrange wrote:
> On Tue, Apr 02, 2013 at 04:22:56PM +0200, Michal Privoznik wrote:
>> ---
>>  src/libvirt_private.syms |  1 +
>>  src/qemu/qemu_domain.c   |  4 +---
>>  src/util/viraudit.c      |  2 +-
>>  src/util/vircommand.c    |  4 ++--
>>  src/util/virerror.c      |  2 +-
>>  src/util/virlog.c        |  2 +-
>>  src/util/virstring.c     | 22 ++++++++++++++++++++--
>>  src/util/virstring.h     |  3 +++
>>  8 files changed, 30 insertions(+), 10 deletions(-)
> 
> This patch is incomplete, missing a number of places which don't report
> errors on enomem. There are probably others that this simple grep does
> not detect too.
> 
> 
> [berrange at avocado libvirt]$ git grep --after 1 virAsprintf  | grep --before 1 ENOMEM
> src/util/vircgroup.c:    if (virAsprintf(&strval, "%llu", value) == -1)
> src/util/vircgroup.c-        return -ENOMEM;
> --
> src/util/vircgroup.c:    if (virAsprintf(&strval, "%lld", value) == -1)
> src/util/vircgroup.c-        return -ENOMEM;
> --
> src/util/vircgroup.c:        if (virAsprintf(&path, "%s/%s", grppath, ent->d_name) == -1) {
> src/util/vircgroup.c-            rc = -ENOMEM;
> --
> src/util/vircgroup.c:    if (virAsprintf(&path, "%s/%s", rootgrp->path, name) < 0) {
> src/util/vircgroup.c-        rc = -ENOMEM;
> --
> src/util/vircgroup.c:    if (virAsprintf(&path, "%s/%s", driver->path, name) < 0)
> src/util/vircgroup.c-        return -ENOMEM;
> --
> src/util/vircgroup.c:    if (virAsprintf(&path, "%s/vcpu%d", driver->path, vcpuid) < 0)
> src/util/vircgroup.c-        return -ENOMEM;
> --
> src/util/vircgroup.c:    if (virAsprintf(&path, "%s/emulator", driver->path) < 0)
> src/util/vircgroup.c-        return -ENOMEM;
> --
> src/util/vircgroup.c:        if (virAsprintf(&subpath, "%s/%s", group->path, ent->d_name) < 0) {
> src/util/vircgroup.c-            rc = -ENOMEM;
> --
> src/util/virdnsmasq.c:    if (virAsprintf(&tmp, "%s.new", path) < 0)
> src/util/virdnsmasq.c-        return -ENOMEM;
> --
> src/util/virdnsmasq.c:    if (virAsprintf(&tmp, "%s.new", path) < 0)
> src/util/virdnsmasq.c-        return -ENOMEM;
> --
> src/util/virpidfile.c:    if (virAsprintf(&procPath, "/proc/%lld/exe", (long long)retPid) < 0) {
> src/util/virpidfile.c-        ret = -ENOMEM;
> [berrange at avocado libvirt]$ git grep --after 1 ALLOC  | grep --before 1 ENOMEM
> src/libvirt.c:    if (VIR_ALLOC(lock) < 0)
> src/libvirt.c-        return ENOMEM;
> --
> src/node_device/node_device_hal.c:        if (VIR_ALLOC(caps) < 0)
> src/node_device/node_device_hal.c-            return ENOMEM;
> --
> src/util/viralloc.c:    if (VIR_ALLOC_VAR_OVERSIZED(struct_size, count, element_size)) {
> src/util/viralloc.c-        errno = ENOMEM;
> --
> src/util/vircgroup.c:    if (VIR_ALLOC((*group)) != 0) {
> src/util/vircgroup.c-        rc = -ENOMEM;
> --
> src/util/vircommand.c:    if (VIR_REALLOC_N(*set, *set_size + 1) < 0) {
> src/util/vircommand.c-        return ENOMEM;
> --
> src/util/virthreadpthread.c:    if (VIR_ALLOC(args) < 0) {
> src/util/virthreadpthread.c-        err = ENOMEM;
> 
> 
> 
> Daniel
> 


Yep, but it makes no harm if we report OOM error (write into logs) in
those cases. Those few cases I am overwriting in my patch are notably
python binding were we do not want to report OOM error anyway, and
virerror.c or virlog.c where reporting could mean infinite recursion.

The others are only those I found by chance. However, thank you for
finding yet another ones - I will look at them in my new round.

Michal