[libvirt] [RFC: PATCH 0/4] Update array allocation macros

[Eric Blake]

I'm posing this series as an RFC for the moment.  The basic premise is
that I got bit trying to use VIR_REALLOC_N when working on the
virCommand API, assuming that it zeroed out new memory because the
documentation said so, only to hit a core dump.  On looking closer, it
became obvious to me that the VIR_REALLOC_N API is deficient - there's
no way to know how much memory, if any, to zeroize without knowing the
original size, so the new memory has to be uninitialized (explaining
my crash), and the documentation is wrong.

So, after some IRC discussions, two ideas emerged in addition to the
obvious documentation update.  One is the simpler VIR_EXPAND_N, where
the array size always matches the array allocation, along with a
VIR_SHRINK_N counterpart which can trim an array back down (and
failure to trim need not be fatal).  But this can scale quadratically
if you end up with a large array, as the existing contents must be
copied on every realloc.  See patches 1 and 2 for an implementation
and use.

The other idea is VIR_RESIZE_N, which tracks allocation separately
from usage, and has better scaling by doing geometric growth only when
needed rather than on every array usage increase.  But it takes more
parameters (making it slightly harder to remember how to use
correctly), along with possible modifications of the struct tracking
an array (so it might not be possible to modify all uses of
VIR_REALLOC_N to use this, if it would end up altering a public struct
layout).  See patches 3 and 4 for an implementation and use.

Note that event.c is an example of code that already tracked
allocation separately from usage, so the new VIR_RESIZE_N was
very easy to use.  On the other hand, libvirtd.c is an example
of code where I had to add allocation tracking; notice the
difference between patches 2/4 and 4/4 of the logic differences
needed when using VIR_EXPAND_N vs. VIR_RESIZE_N.  And notice
that both patch 2 and patch 4 provide a net reduction in lines
outside of memory.[ch].

Either way, I'm willing to audit all remaining uses of VIR_REALLOC_N
(in fact, libvirtd.c has another use of VIR_REALLOC_N to allocate
a buffer for getgrnam_r, where the uninitialized memory aspect
was just fine and not worth converting).  The only reason I'm posting
this series as an RFC is to get a feel for which of the two styles
I should prefer when doing my audit on the rest of the code base
(rather than doing style A now only to find that style B would be
preferred).

Also, as written, the implementation forces all array count variables
to be size_t if using VIR_EXPAND_N or VIR_RESIZE_N; I may come across
a public API where I can't change the array count type, in which case
I will have to rewrite the macros to be type-agnostic.  Well, more
like making the macro choose between a 4-byte or 8-byte
implementation, based on sizeof(count), and adding a compiler verify()
that count must be one of those two sizes, whether on 32-bit or 64-bit
machines.

Eric Blake (4):
  memory: make it safer to expand arrays
  daemon: use safer memory growth macros
  memory: make it easier to avoid quadratic scaling of arrays
  daemon: use more efficient memory growth macros

 daemon/event.c       |   44 ++++++++++-------------
 daemon/libvirtd.c    |    8 ++---
 daemon/libvirtd.h    |   11 +++---
 docs/hacking.html.in |   61 ++++++++++++++++++++++++++------
 src/util/memory.c    |   94 +++++++++++++++++++++++++++++++++++++++++++++++++-
 src/util/memory.h    |   77 +++++++++++++++++++++++++++++++++++++---
 6 files changed, 242 insertions(+), 53 deletions(-)

-- 
1.7.2.1