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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@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@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

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]