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

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