On 5/3/19 11:01 AM, Erik Skultety wrote:
On Thu, Apr 18, 2019 at 02:11:25PM +0200, Michal Privoznik wrote:
Currently, the way virBufferFreeAndReset() works is it relies on
virBufferContentAndReset() to fetch the buffer content which is
then freed. This works as long as there is no bug in virBuffer*
implementation (not true apparently). Explicitly call free() over
buffer content.
Signed-off-by: Michal Privoznik <mpriv...@redhat.com>
---
src/util/virbuffer.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/src/util/virbuffer.c b/src/util/virbuffer.c
index b2ae7963a1..ac03b15a61 100644
--- a/src/util/virbuffer.c
+++ b/src/util/virbuffer.c
@@ -281,9 +281,8 @@ virBufferContentAndReset(virBufferPtr buf)
*/
void virBufferFreeAndReset(virBufferPtr buf)
{
- char *str = virBufferContentAndReset(buf);
-
- VIR_FREE(str);
+ if (buf)
+ virBufferSetError(buf, 0);
You saved 1 call to memset. I also don't see how we can break
virBufferContentAndReset in a way that we couldn't mess up virBufferSetError in
the same way. Additionally, seeing a call to virBufferSetError in a free helper
looks suspicious (even if it has a '0' as argument). If anything, I'd rename
virBuggerFreeAndReset to virBufferClean.
Thing is, if 1/3 wasn't merged, then the test I'm introducing there
would leak memory. But not with this patch merged. So from long term
perspective, if there is ever some similar bug we won't leak any memory.
Yeah, naming is hard.
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
