On Tue, 2019-02-05 at 15:23 +0000, Daniel P. Berrangé wrote:
On Tue, Feb 05, 2019 at 04:16:21PM +0100, Andrea Bolognani wrote:
> The memory allocated by VIR_REALLOC_N() is uninitialized,
> which means it's not possible to figure out whether any
> output was produced at all after the fact.
I really wish we had never added the VIR_REALLOC_N function.
One of the best things about VIR_ALLOC/VIR_EXPAND/VIR_RESIZE
are that they remove all bugs related to use of uninitialized
memory. We really ought to try to eliminate use of VIR_REALLOC_N
in favour of the other safer functions throughout the code.
That should be reasonably simple to do, since the functions we'd be
moving to offer more guarantees than the ones we'd be moving away
from. There are quite a few call sites, though :)
[...]
> @@ -2057,11 +2057,13 @@ virCommandProcessIO(virCommandPtr cmd)
> outfd = cmd->outfd;
> if (VIR_REALLOC_N(*cmd->outbuf, 1) < 0)
> ret = -1;
> + *cmd->outbuf[0] = '\0';
> }
> if (cmd->errbuf) {
> errfd = cmd->errfd;
> if (VIR_REALLOC_N(*cmd->errbuf, 1) < 0)
> ret = -1;
> + *cmd->errbuf[0] = '\0';
Here we don't really care about the original contents on outbuf/errbuf.
I'd probably go for making that explicit by replacing VIR_REALLOC_N
with VIR_FREE + VIR_ALLOC_N
Sure, can do.
--
Andrea Bolognani / Red Hat / Virtualization