Re: [libvirt PATCH 4/4] tools: be more paranoid about possibly NULL description

2020-07-22 Thread Laine Stump
On 7/22/20 1:21 PM, Daniel P. Berrangé wrote: GCC 10 complains about "desc" possibly being a NULL dereference. Even though it is a false positive, we can easily avoid it. Signed-off-by: Daniel P. Berrangé Reviewed-by: Laine Stump So those were the only complaints of gcc 10?

Re: [libvirt PATCH 3/4] tests: don't mock the time() function on mingw

2020-07-22 Thread Laine Stump
system now though, this just stubs out the offending function. Signed-off-by: Daniel P. Berrangé Reviewed-by: Laine Stump --- tests/virnetdaemonmock.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/virnetdaemonmock.c b/tests/virnetdaemonmock.c index 3b92fff8c9..c523da0791

Re: [libvirt PATCH 1/4] util: refactor code to workaround gcc 10.1.0 bug

2020-07-22 Thread Laine Stump
n ack for this when you posted it the first time this morning, but I guess it failed to send, so it was still sitting in an obscured window on my screen...) I don't have a system running gcc 10 yet, but this code looks to provide identical functionality, and still works with gcc 9.3.1 Reviewed-by:

Re: [libvirt PATCH 2/4] m4: enable -fstack-protector-strong on mingw

2020-07-22 Thread Laine Stump
that right?) Reviewed-by: Laine Stump Signed-off-by: Daniel P. Berrangé --- m4/virt-compile-warnings.m4 | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/m4/virt-compile-warnings.m4 b/m4/virt-compile-warnings.m4 index d3538d59f8..d171d09991 100644 --- a/m4/virt-compile-w

Re: [libvirt PATCH v2 06/15] network: eliminate unnecessary labels

2020-07-21 Thread Laine Stump
On 7/21/20 8:04 AM, John Ferlan wrote: On 7/7/20 5:08 PM, Laine Stump wrote: All these cleanup/error labels were reduced to having just "return ret" by a previous patch, so get rid of them and return directly. Signed-off-by: Laine Stump --- src/network/bridge_driver.c

Re: [libvirt PATCH v2 08/15] nwfilter: remove unnecessary code from ebtablesGetSubChainInsts()

2020-07-20 Thread Laine Stump
On 7/20/20 5:04 PM, Ján Tomko wrote: On a Saturday in 2020, Laine Stump wrote: On 7/15/20 11:30 AM, Ján Tomko wrote: On a Tuesday in 2020, Laine Stump wrote: Signed-off-by: Laine Stump My S-o-b stands. I still think this is the right thing to do. S-o-b merely means that you

[PATCH 3/5] util: log an error if virXMLNodeContentString will return NULL

2020-07-20 Thread Laine Stump
to (since it would be the same error message for all of them anyway). Signed-off-by: Laine Stump --- src/util/virxml.c | 18 +- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/src/util/virxml.c b/src/util/virxml.c index 27d22598ee..5315d4ff6f 100644 --- a/src/util/virxml.c

[PATCH 1/5] conf: refactor virDomainBlkioDeviceParseXML to reduce calls to xmlNodeGetContent

2020-07-20 Thread Laine Stump
bypassing the "node = node->next". Signed-off-by: Laine Stump Change from V1: turned into for() loop and log error rather than ignoring NULL. Jano had suggested we might be able to set dev->path directly instead of using a temporary var, but doing that would require keepin

[FYI PATCH 5/5] util: open code virXMLNodeContentString to access the node object directly

2020-07-20 Thread Laine Stump
layout of the XML node object list are incorrect. Note that while calling virXMLNodeContentString() would return NULL from an OOM situation, this new code will exit the process on OOM (since it is calling glib for memory allocation). Signed-off-by: Laine Stum

[PATCH 2/5] util: replace all calls to xmlNodeGetContent with virXMLNodeContentString

2020-07-20 Thread Laine Stump
No functional change Signed-off-by: Laine Stump --- src/conf/domain_conf.c | 20 ++-- src/conf/network_conf.c | 2 +- src/conf/node_device_conf.c | 4 ++-- 3 files changed, 13 insertions(+), 13 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c

[PATCH 0/5] be consistent about error checking xmlNodeGetContent() return

2020-07-20 Thread Laine Stump
;RFC"), since really all it changes is that libvirt will exit on OOM, and log (different, but not any more informative) error messages when the problem isn't OOM. Unless someone has a strong opinion otherwise, I think just the first 4 patches should be applied, and users can just "deal" wit

[PATCH 4/5] treat all NULL returns from virXMLNodeContentString() as an error

2020-07-20 Thread Laine Stump
and stop erroneously equating NULL with "". The latter means that the element has empty content, while the former means there was an error during parsing (either internal with the parser, or the content of the XML was bad). Signed-off-by: Laine Stump --- src/conf/domain_conf.c

Re: [libvirt PATCH v2 08/15] nwfilter: remove unnecessary code from ebtablesGetSubChainInsts()

2020-07-17 Thread Laine Stump
On 7/15/20 11:30 AM, Ján Tomko wrote: On a Tuesday in 2020, Laine Stump wrote: On failure, this function would clear out and free the list of subchains it had been called with. This is unnecessary, because the *only* caller of this function will also clear out and free the list of subchains

[PATCH] network: refactor networkSetIPv6Sysctls() for proper g_autofree usage

2020-07-17 Thread Laine Stump
This function used the same char* three times for different purposes, freeing it after each use. Since we don't want to ever manually free an autofree'd pointer, modify it to use three separate char*, and make them all g_autofree. Signed-off-by: Laine Stump --- This was suggested by Jan

Re: [libvirt PATCH v2 05/15] network: use g_auto wherever appropriate

2020-07-15 Thread Laine Stump
On 7/15/20 11:10 AM, Ján Tomko wrote: On a Tuesday in 2020, Laine Stump wrote: This includes standard g_autofree() as well as other objects that have a cleanup function defined to use via g_autoptr (virCommand, virJSONValue) Signed-off-by: Laine Stump --- src/network/bridge_driver.c

Re: [libvirt PATCH v2 00/15] convert network and nwfilter directories to glib memory allocation.

2020-07-14 Thread Laine Stump
ping On 7/7/20 5:08 PM, Laine Stump wrote: V1 was here: https://www.redhat.com/archives/libvir-list/2020-June/msg01156.html Some patches were ACKed and pushed. I re-ordered/re-organized most of the rest, and removed some others to deal with separately (the xmlNodeContent stuff) What's left

[PATCH] docs: point out that locals should be defined at the top of a block of code

2020-07-09 Thread Laine Stump
.html Signed-off-by: Laine Stump --- docs/coding-style.rst | 38 ++ 1 file changed, 38 insertions(+) diff --git a/docs/coding-style.rst b/docs/coding-style.rst index 03b89c86e5..b9b4a16987 100644 --- a/docs/coding-style.rst +++ b/docs/coding-style.rst @@ -541,6

Re: [libvirt PATCH] All pointers to virXMLPropString() use g_autofree.

2020-07-08 Thread Laine Stump
le.html) about it, and it may just be leftover from a time when we supported a compiler that required all declarations to be at top of scope, I think pretty much all of libvirts code declares all local variables at the top of the scope. That's simple enough for me to just fixu

Re: How to best handle NULL return from xmlNodeGetContent()

2020-07-08 Thread Laine Stump
On 7/8/20 4:35 AM, Daniel P. Berrangé wrote: On Tue, Jul 07, 2020 at 06:48:57PM -0400, Laine Stump wrote: libvirt has several uses of xmlNodeGetContent() (from libxml2) added at different times over the years. Some of those uses report an Out of Memory error when xmlNodeGetContent() returns

How to best handle NULL return from xmlNodeGetContent()

2020-07-07 Thread Laine Stump
libvirt has several uses of xmlNodeGetContent() (from libxml2) added at different times over the years. Some of those uses report an Out of Memory error when xmlNodeGetContent() returns NULL, and some of them ignore a NULL return (treating it as if it were ""), and some just assume that the

[libvirt PATCH v2 07/15] network: use g_free() in place of remaining VIR_FREE()

2020-07-07 Thread Laine Stump
Signed-off-by: Laine Stump --- src/network/bridge_driver.c | 45 +++-- 1 file changed, 23 insertions(+), 22 deletions(-) diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 79b2ca3330..7d81d4dd78 100644 --- a/src/network/bridge_driver.c

[libvirt PATCH v2 15/15] nwfilter: convert remaining VIR_FREE() to g_free()

2020-07-07 Thread Laine Stump
Signed-off-by: Laine Stump --- src/nwfilter/nwfilter_dhcpsnoop.c | 16 src/nwfilter/nwfilter_driver.c| 10 +- src/nwfilter/nwfilter_ebiptables_driver.c | 2 +- src/nwfilter/nwfilter_gentech_driver.c| 6 +++--- src/nwfilter/nwfilter_learnipaddr.c

[libvirt PATCH v2 01/15] replace g_new() with g_new0() for consistency

2020-07-07 Thread Laine Stump
.) Signed-off-by: Laine Stump --- src/qemu/qemu_backup.c | 2 +- src/util/virutil.c | 2 +- tests/qemuhotplugmock.c | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_backup.c b/src/qemu/qemu_backup.c index 8dc9d2504d..dae9300567 100644 --- a/src/qemu

[libvirt PATCH v2 05/15] network: use g_auto wherever appropriate

2020-07-07 Thread Laine Stump
This includes standard g_autofree() as well as other objects that have a cleanup function defined to use via g_autoptr (virCommand, virJSONValue) Signed-off-by: Laine Stump --- src/network/bridge_driver.c | 206 ++ src/network/bridge_driver_linux.c | 7

[libvirt PATCH v2 12/15] nwfilter: use standard label names when reasonable

2020-07-07 Thread Laine Stump
Rather than having labels named exit, done, exit_snooprequnlock, skip_rename, etc, use the standard "cleanup" label. And instead of err_exit, malformed, tear_down_tmpebchains, use "error". Signed-off-by: Laine Stump --- src/nwfilter/nwfilter_dhcpsnoop.c | 36

[libvirt PATCH v2 13/15] nwfilter: replace VIR_ALLOC with g_new0

2020-07-07 Thread Laine Stump
Signed-off-by: Laine Stump --- src/nwfilter/nwfilter_dhcpsnoop.c | 9 +++-- src/nwfilter/nwfilter_driver.c| 3 +-- src/nwfilter/nwfilter_ebiptables_driver.c | 3 +-- src/nwfilter/nwfilter_gentech_driver.c| 3 +-- src/nwfilter/nwfilter_learnipaddr.c | 6 ++ 5

[libvirt PATCH v2 00/15] convert network and nwfilter directories to glib memory allocation.

2020-07-07 Thread Laine Stump
* re-order to replace VIR_ALLOC first (without adding any g_auto*) instead of doing it after g_auto conversion of automatics, then do all g_auto additions at o * separate label elimination into separate patches per jtomko's suggestion. Laine Stump (15): replace g_new() w

[libvirt PATCH v2 14/15] nwfilter: convert local pointers to use g_auto*

2020-07-07 Thread Laine Stump
Signed-off-by: Laine Stump --- src/nwfilter/nwfilter_dhcpsnoop.c | 91 +++ src/nwfilter/nwfilter_ebiptables_driver.c | 75 +++ src/nwfilter/nwfilter_gentech_driver.c| 15 ++-- src/nwfilter/nwfilter_learnipaddr.c | 7 +- 4 files changed, 61

[libvirt PATCH v2 10/15] nwfilter: define a typedef for struct ebtablesSubChainInst

2020-07-07 Thread Laine Stump
Signed-off-by: Laine Stump --- src/nwfilter/nwfilter_ebiptables_driver.c | 14 -- 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/nwfilter/nwfilter_ebiptables_driver.c b/src/nwfilter/nwfilter_ebiptables_driver.c index 426212e0dc..cc0f3f93d9 100644 --- a/src

[libvirt PATCH v2 09/15] nwfilter: clear nrules when resetting virNWFilterInst

2020-07-07 Thread Laine Stump
It's possible/probable the callers to virNWFilterInstReset() make it unnecessary to set the object's nrules to 0 after freeing all its rules, but that same function is setting nfilters to 0, so let's do the same for the sake of consistency. Signed-off-by: Laine Stump --- src/nwfilter

[libvirt PATCH v2 11/15] nwfilter: transform logic in virNWFilterRuleInstSort to eliminate label

2020-07-07 Thread Laine Stump
This rewrite of a nested conditional produces the same results, but eliminate a goto and corresponding label. Signed-off-by: Laine Stump --- src/nwfilter/nwfilter_ebiptables_driver.c | 11 +-- 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/nwfilter

[libvirt PATCH v2 08/15] nwfilter: remove unnecessary code from ebtablesGetSubChainInsts()

2020-07-07 Thread Laine Stump
sense for the function that is creating the entire list to be the one freeing the entire list, rather than having a function whose purpose is only to create *one item* on the list freeing the entire list). Signed-off-by: Laine Stump --- src/nwfilter/nwfilter_ebiptables_driver.c | 6 -- 1 file

[libvirt PATCH v2 04/15] network: replace VIR_ALLOC/REALLOC with g_new0/g_renew

2020-07-07 Thread Laine Stump
Signed-off-by: Laine Stump --- src/network/bridge_driver.c | 29 ++--- 1 file changed, 10 insertions(+), 19 deletions(-) diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 713763130b..ab359acdb5 100644 --- a/src/network/bridge_driver.c +++ b/src

[libvirt PATCH v2 03/15] define g_autoptr cleanup function for virNetworkDHCPLease

2020-07-07 Thread Laine Stump
virNetworkDHCPLease and virNetworkDHCPLeaseFree() are declared in the public API file libvirt-network.h, and we can't pollute that with glib macro invocations, so put this in src/datatypes.h next to the other virNetwork items. Signed-off-by: Laine Stump --- src/datatypes.h | 2 ++ 1 file

[libvirt PATCH v2 02/15] util: define g_autoptr cleanups for a couple dnsmasq objects

2020-07-07 Thread Laine Stump
Signed-off-by: Laine Stump --- src/util/virdnsmasq.h | 4 1 file changed, 4 insertions(+) diff --git a/src/util/virdnsmasq.h b/src/util/virdnsmasq.h index 4c14bc6ca7..e3814c2eb1 100644 --- a/src/util/virdnsmasq.h +++ b/src/util/virdnsmasq.h @@ -78,10 +78,14 @@ typedef enum { typedef

[libvirt PATCH v2 06/15] network: eliminate unnecessary labels

2020-07-07 Thread Laine Stump
All these cleanup/error labels were reduced to having just "return ret" by a previous patch, so get rid of them and return directly. Signed-off-by: Laine Stump --- src/network/bridge_driver.c | 264 -- src/network/bridge_driver_linux.c | 15 +- 2 fil

Re: Release of libvirt-6.5.0

2020-07-07 Thread Laine Stump
On 7/3/20 3:56 AM, Daniel Veillard wrote: It will also be my last release of libvirt after close to 15 years, . (I missed this sentence when I saw the mail the first time, and just now it randomly popped up when scrolling through messages.) Thanks for your helpful and positive attitude,

[PATCH 14/32] network: use g_auto() for all virBuffers

2020-07-05 Thread Laine Stump
Signed-off-by: Laine Stump --- src/network/bridge_driver.c | 9 +++-- 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 0f5212ce04..9f37d8f558 100644 --- a/src/network/bridge_driver.c +++ b/src/network

[PATCH 21/32] conf: eliminate unnecessary labels

2020-07-05 Thread Laine Stump
Signed-off-by: Laine Stump --- src/conf/capabilities.c| 5 +-- src/conf/checkpoint_conf.c | 8 ++-- src/conf/cpu_conf.c| 18 +++- src/conf/domain_conf.c | 88 +++--- src/conf/network_conf.c| 5 +-- src/conf/nwfilter_conf.c | 5

[PATCH 31/32] use g_autoptr() for all usages of virFirewallNew/Free

2020-07-05 Thread Laine Stump
Signed-off-by: Laine Stump --- src/network/bridge_driver_linux.c | 11 ++--- src/nwfilter/nwfilter_ebiptables_driver.c | 31 -- src/util/virebtables.c| 8 +--- src/util/viriptables.c| 6 +-- tests/virfirewalltest.c

[PATCH 28/32] esx: eliminate unnecessary labels

2020-07-05 Thread Laine Stump
Signed-off-by: Laine Stump --- src/esx/esx_vi.c | 9 +++-- 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/esx/esx_vi.c b/src/esx/esx_vi.c index d48a24e9d3..71aa3876b3 100644 --- a/src/esx/esx_vi.c +++ b/src/esx/esx_vi.c @@ -369,7 +369,7 @@ int esxVI_CURL_Download

[PATCH 12/32] rpc: use g_auto() for all virBuffers

2020-07-05 Thread Laine Stump
Signed-off-by: Laine Stump --- src/rpc/virnetclient.c| 4 ++-- src/rpc/virnetlibsshsession.c | 7 ++- src/rpc/virnetsocket.c| 2 +- src/rpc/virnetsshsession.c| 2 +- 4 files changed, 6 insertions(+), 9 deletions(-) diff --git a/src/rpc/virnetclient.c b/src/rpc

[PATCH 10/32] util: use g_auto() for all virBuffers

2020-07-05 Thread Laine Stump
Signed-off-by: Laine Stump --- src/util/virbitmap.c | 4 ++-- src/util/vircommand.c | 3 +-- src/util/virconf.c| 5 ++--- src/util/virdnsmasq.c | 6 ++ src/util/virfile.c| 2 +- src/util/virfilecache.c | 2 +- src/util/virfirewall.c| 2 +- src/util

[PATCH 23/32] util: eliminate unnecessary labels

2020-07-05 Thread Laine Stump
Signed-off-by: Laine Stump --- src/util/virqemu.c | 8 ++-- src/util/virsocketaddr.c | 22 +++--- src/util/virxml.c| 8 ++-- 3 files changed, 11 insertions(+), 27 deletions(-) diff --git a/src/util/virqemu.c b/src/util/virqemu.c index 52f3a2ca12

[PATCH 19/32] libxml: eliminate extra copy of string

2020-07-05 Thread Laine Stump
with it. Signed-off-by: Laine Stump --- src/libxl/libxl_conf.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index 124e08d598..fe8ad4a3cb 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -1335,7 +1335,7 @@ libxlMakeNic

[PATCH 20/32] bhyve: eliminate unnecessary labels

2020-07-05 Thread Laine Stump
Signed-off-by: Laine Stump --- src/bhyve/bhyve_command.c | 25 ++--- 1 file changed, 10 insertions(+), 15 deletions(-) diff --git a/src/bhyve/bhyve_command.c b/src/bhyve/bhyve_command.c index 9649c2d2a2..22d0b24ec4 100644 --- a/src/bhyve/bhyve_command.c +++ b/src/bhyve

[PATCH 09/32] conf: use g_auto() for all virBuffers

2020-07-05 Thread Laine Stump
Signed-off-by: Laine Stump --- src/conf/capabilities.c | 8 +++- src/conf/checkpoint_conf.c | 2 +- src/conf/cpu_conf.c | 9 +++-- src/conf/domain_addr.c | 2 +- src/conf/domain_capabilities.c | 2 +- src/conf/domain_conf.c | 12

[PATCH 30/32] storage: eliminate unnecessary labels

2020-07-05 Thread Laine Stump
Signed-off-by: Laine Stump --- src/storage/storage_util.c | 5 + 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c index 36b5b21a5b..9f46ea764b 100644 --- a/src/storage/storage_util.c +++ b/src/storage/storage_util.c

[PATCH 18/32] remove redundant calls to virBufferFreeAndReset()

2020-07-05 Thread Laine Stump
virBuffers are auto-freed there is no reason for the lower level functions like these to spend time freeing a buffer that is guaranteed to be freed momentarily anyway. Signed-off-by: Laine Stump --- src/conf/checkpoint_conf.c | 1 - src/conf/domain_conf.c | 1 - src/conf/snapshot_conf.c

[PATCH 25/32] tools: eliminate unnecessary labels

2020-07-05 Thread Laine Stump
Signed-off-by: Laine Stump --- tools/virsh-pool.c | 13 ++--- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/tools/virsh-pool.c b/tools/virsh-pool.c index 885e000ed2..622b1396d0 100644 --- a/tools/virsh-pool.c +++ b/tools/virsh-pool.c @@ -330,9 +330,10 @@ virshBuildPoolXML

[PATCH 16/32] qemu: remove unnecessary virBufferFreeAndReset() after virCommandAddArgBuffer()

2020-07-05 Thread Laine Stump
The latter function is guaranteed to always clear out the virBuffer anyway, so this is redundant and could add to extra cargo-cult code if used as an example. Signed-off-by: Laine Stump --- src/qemu/qemu_command.c | 1 - 1 file changed, 1 deletion(-) diff --git a/src/qemu/qemu_command.c b/src

[PATCH 29/32] nwfilter: eliminate unnecessary labels

2020-07-05 Thread Laine Stump
Signed-off-by: Laine Stump --- src/nwfilter/nwfilter_ebiptables_driver.c | 89 +++ 1 file changed, 43 insertions(+), 46 deletions(-) diff --git a/src/nwfilter/nwfilter_ebiptables_driver.c b/src/nwfilter/nwfilter_ebiptables_driver.c index dad631f03b..6cdb3ca45e 100644

[PATCH 11/32] cpu: use g_auto() for all virBuffers

2020-07-05 Thread Laine Stump
Signed-off-by: Laine Stump --- src/cpu/cpu_map.c | 2 +- src/cpu/cpu_x86.c | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/cpu/cpu_map.c b/src/cpu/cpu_map.c index 4465ebfa7b..d14488f8aa 100644 --- a/src/cpu/cpu_map.c +++ b/src/cpu/cpu_map.c @@ -171,7 +171,7 @@ int

[PATCH 24/32] tests: eliminate unnecessary labels

2020-07-05 Thread Laine Stump
Signed-off-by: Laine Stump --- tests/virbuftest.c | 35 ++- 1 file changed, 14 insertions(+), 21 deletions(-) diff --git a/tests/virbuftest.c b/tests/virbuftest.c index 39ae7c2b9d..df341be50c 100644 --- a/tests/virbuftest.c +++ b/tests/virbuftest.c @@ -100,7

[PATCH 04/32] libxl: use g_auto() for all virBuffers

2020-07-05 Thread Laine Stump
Signed-off-by: Laine Stump --- src/libxl/libxl_conf.c | 6 ++ src/libxl/libxl_driver.c| 2 +- src/libxl/libxl_migration.c | 2 +- src/libxl/xen_common.c | 12 +--- src/libxl/xen_xl.c | 19 +++ src/libxl/xen_xm.c | 3 +-- 6 files

[PATCH 07/32] tests: use g_auto for all virBuffers

2020-07-05 Thread Laine Stump
Signed-off-by: Laine Stump --- tests/commandtest.c | 3 +-- tests/cputest.c | 2 +- tests/networkxml2firewalltest.c | 3 +-- tests/nodedevmdevctltest.c | 6 ++ tests/nwfilterebiptablestest.c | 21 +++-- tests

[PATCH 17/32] conf: consistently check for error when calling virSysinfoFormat()

2020-07-05 Thread Laine Stump
Every other caller of this function checks for an error return and ends their formatting early if there is an error. This function happily continues on its way. Signed-off-by: Laine Stump --- src/conf/domain_conf.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src

[PATCH 32/32] eliminate unnecessary labels and ret variables

2020-07-05 Thread Laine Stump
after making all virFirewall objects use g_autoptr(). Signed-off-by: Laine Stump --- src/network/bridge_driver_linux.c | 27 +++ src/nwfilter/nwfilter_ebiptables_driver.c | 32 +++ src/util/virebtables.c| 16 ++-- src/util

[PATCH 13/32] nwfilter: use g_auto() for all virBuffers

2020-07-05 Thread Laine Stump
Signed-off-by: Laine Stump --- src/nwfilter/nwfilter_ebiptables_driver.c | 8 +++- src/nwfilter/nwfilter_gentech_driver.c| 6 ++ src/nwfilter/nwfilter_learnipaddr.c | 2 +- 3 files changed, 6 insertions(+), 10 deletions(-) diff --git a/src/nwfilter/nwfilter_ebiptables_driver.c

[PATCH 03/32] hyperv: use g_auto() for all virBuffers

2020-07-05 Thread Laine Stump
Signed-off-by: Laine Stump --- src/hyperv/hyperv_driver.c | 34 -- src/hyperv/hyperv_wmi.c| 11 +-- 2 files changed, 21 insertions(+), 24 deletions(-) diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c index 4677a25ff8..20d372b274

[PATCH 26/32] network: eliminate unnecessary labels

2020-07-05 Thread Laine Stump
Signed-off-by: Laine Stump --- src/network/bridge_driver.c | 16 ++-- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 9f37d8f558..713763130b 100644 --- a/src/network/bridge_driver.c +++ b/src/network

[PATCH 22/32] libxl: eliminate unnecessary labels

2020-07-05 Thread Laine Stump
Signed-off-by: Laine Stump --- src/libxl/libxl_conf.c | 11 --- src/libxl/xen_common.c | 16 +--- src/libxl/xen_xl.c | 25 - src/libxl/xen_xm.c | 7 ++- 4 files changed, 19 insertions(+), 40 deletions(-) diff --git a/src/libxl

[PATCH 08/32] tools: use g_auto() for all virBuffers

2020-07-05 Thread Laine Stump
Signed-off-by: Laine Stump --- tools/virsh-checkpoint.c | 3 +- tools/virsh-domain-monitor.c | 3 +- tools/virsh-domain.c | 58 +--- tools/virsh-pool.c | 6 ++-- tools/virsh-secret.c | 2 +- tools/virsh-snapshot.c | 3

[PATCH 15/32] use g_auto() for all remaining non-g_auto() virBuffers

2020-07-05 Thread Laine Stump
Signed-off-by: Laine Stump --- src/hypervisor/domain_driver.c | 7 +++ src/locking/lock_driver_sanlock.c | 2 +- src/node_device/node_device_udev.c | 2 +- src/openvz/openvz_driver.c | 5 ++--- src/security/virt-aa-helper.c | 4 ++-- src/storage/storage_backend_rbd.c | 7

[PATCH 27/32] lxc: eliminate unnecessary labels

2020-07-05 Thread Laine Stump
Signed-off-by: Laine Stump --- src/lxc/lxc_controller.c | 9 +++-- 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c index 01cdeb29db..ae6b737b60 100644 --- a/src/lxc/lxc_controller.c +++ b/src/lxc/lxc_controller.c @@ -1380,13

[PATCH 06/32] qemu: use g_auto() for all virBuffers

2020-07-05 Thread Laine Stump
Signed-off-by: Laine Stump --- src/qemu/qemu_agent.c| 2 +- src/qemu/qemu_block.c| 2 +- src/qemu/qemu_capabilities.c | 2 +- src/qemu/qemu_command.c | 4 ++-- src/qemu/qemu_dbus.c | 2 +- src/qemu/qemu_domain.c | 4 ++-- src/qemu

[PATCH 00/32] always use g_auto() for virBuffer and virFirewall objects

2020-07-05 Thread Laine Stump
example from my notes: virStoragePoolSaveState calls virStoragePoolDefFormatBuf which calls virStoragePoolSourceFormat, which errors, returns virStoragePoolDefFormatBuf, returns virStoragePoolSaveState returns without freeing virBuffer Laine Stump (32): bhyve: use g_auto() for all virBuffe

[PATCH 05/32] lxc: use g_auto() for all virBuffers

2020-07-05 Thread Laine Stump
Signed-off-by: Laine Stump --- src/lxc/lxc_container.c | 4 +--- src/lxc/lxc_controller.c | 3 +-- src/lxc/lxc_driver.c | 2 +- src/lxc/lxc_fuse.c | 3 +-- 4 files changed, 4 insertions(+), 8 deletions(-) diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index c22b7b0709

[PATCH 01/32] bhyve: use g_auto() for all virBuffers

2020-07-05 Thread Laine Stump
infoFormat() to clear out the buffer when there is an error). I think this is sloppy behavior, and that the toplevel function that defines and initializes the buffer should be the function clearing it at the end. Signed-off-by: Laine Stump --- src/bhyve/bhyve_command.c | 15 ++- src/bhyve/bhyv

[PATCH 02/32] esx: use g_auto() for all virBuffers

2020-07-05 Thread Laine Stump
Signed-off-by: Laine Stump --- src/esx/esx_driver.c | 19 +-- src/esx/esx_util.c | 4 ++-- src/esx/esx_vi.c | 19 +-- src/esx/esx_vi_methods.c | 6 +- 4 files changed, 13 insertions(+), 35 deletions(-) diff --git a/src/esx/esx_driver.c b

Re: [PATCH 04/25] util: validate return from xmlNodeGetContent before use

2020-06-26 Thread Laine Stump
On 6/25/20 6:55 PM, Ján Tomko wrote: On a Wednesday in 2020, Laine Stump wrote: There were a few uses of xmlNodeGetContent() that didn't check for NULL before using the result. A NULL return from xmlNodeGetContent() *could* (probably does) mean that there was an Out of Memory condition

Re: [PATCH 03/25] conf: refactor virDomainBlkioDeviceParseXML to remove possible NULL dereference

2020-06-26 Thread Laine Stump
On 6/26/20 6:54 PM, Laine Stump wrote: On 6/25/20 6:44 PM, Ján Tomko wrote: and give a possibility of assigning the path directly to 'path', without the extra steal_pointer. I don't follow there - if you assign directly from xmlNodeGetContent() into path, then you'll need to duplicate

Re: [PATCH 03/25] conf: refactor virDomainBlkioDeviceParseXML to remove possible NULL dereference

2020-06-26 Thread Laine Stump
On 6/25/20 6:44 PM, Ján Tomko wrote: On a Wednesday in 2020, Laine Stump wrote: virDomainBlkioDeviceParseXML() has multiple cases of sending the return from xmlNodeGetContent() directly to virStrToLong_xx() without checking for NULL. Although it is *very* rare for xmlNodeGetContent() to return

Re: [PATCH 11/25] network: use g_free() in place of remaining VIR_FREE()

2020-06-26 Thread Laine Stump
On 6/25/20 11:12 AM, Daniel P. Berrangé wrote: On Thu, Jun 25, 2020 at 11:01:48AM -0400, Laine Stump wrote: On 6/25/20 3:55 AM, Peter Krempa wrote: On Wed, Jun 24, 2020 at 23:34:00 -0400, Laine Stump wrote: Signed-off-by: Laine Stump --- src/network/bridge_driver.c | 59

Re: [libvirt PATCH v3 0/2] Refuse PCI Address for ramfb device definition

2020-06-25 Thread Laine Stump
-display-device-pci-address.xml For both: Reviewed-by: Laine Stump and pushed (...nd I just realized I forgot to add the Reviewed-by: tag before pushing :-/. Ah well, it shows me as the committer, so you won't have to *totally* take the fall if something blows up :-))

Re: [PATCH 16/25] squash into 'network: convert local pointers to g_auto*'

2020-06-25 Thread Laine Stump
On 6/25/20 7:34 PM, Ján Tomko wrote: On a Wednesday in 2020, Laine Stump wrote: OOPS!! I meant to squash this into patch 10 before posting. If you want to just review it separately I can squash it in before push. Or if you want to be pedantic I can squash it in and resend :-) To me

Re: [PATCH 07/25] util: eliminate error label in virDomainDefFormatInternalSetRootName()

2020-06-25 Thread Laine Stump
On 6/25/20 7:08 PM, Ján Tomko wrote: On a Wednesday in 2020, Laine Stump wrote: The only reason for the error label in this function is to call virBufferFreeAndReset(). It's actually more common for a failed format function to just leave the virBuffer alone and let the caller free it when

Re: [PATCH 09/25] util: add g_autoptr cleanup function for virFirewall objects

2020-06-25 Thread Laine Stump
On 6/25/20 7:17 PM, Ján Tomko wrote: The cleanup function was already added by: commit 2ad0284627ea3d6c123e0a266b9c7bb00aea4576 CommitDate: 2018-07-27 17:21:04 +0200     util: firewall: define cleanup function using VIR_DEFINE_AUTOPTR_FUNC On a Wednesday in 2020, Laine Stump wrote: Put

Re: [libvirt PATCH v2] qemu: ramfb video device doesn't support PCI address

2020-06-25 Thread Laine Stump
On 6/25/20 12:30 PM, Jonathon Jongsma wrote: On Thu, 2020-06-25 at 12:20 -0400, Laine Stump wrote: On 6/25/20 10:34 AM, Jonathon Jongsma wrote: Although a ramfb video device is not a PCI device, we don't currently report an error for ramfb device definitions containing a PCI address. However

Re: [libvirt PATCH v2] qemu: ramfb video device doesn't support PCI address

2020-06-25 Thread Laine Stump
_ERROR("video-ramfb-display-device-pci-address"); Did you forget to git-add the test case data? DO_TEST("video-none-device", QEMU_CAPS_VNC); DO_TEST_PARSE_ERROR("video-invalid-multiple-devices", NONE); With the test case data added Reviewed-by: Laine Stump

Re: [PATCH 11/25] network: use g_free() in place of remaining VIR_FREE()

2020-06-25 Thread Laine Stump
On 6/25/20 3:55 AM, Peter Krempa wrote: On Wed, Jun 24, 2020 at 23:34:00 -0400, Laine Stump wrote: Signed-off-by: Laine Stump --- src/network/bridge_driver.c | 59 + 1 file changed, 33 insertions(+), 26 deletions(-) diff --git a/src/network

Re: [PATCH] qemuDomainDeviceNetDefPostParse: Switch order of conditions

2020-06-25 Thread Laine Stump
domain XML). If we switch the order of these two checks then the short circuit evaluation will ensure the expensive check is done only if really needed. Oops! I should have caught that when I reviewed the earlier commit. Reviewed-by: Laine Stump

Re: [PATCH 16/25] squash into 'network: convert local pointers to g_auto*'

2020-06-24 Thread Laine Stump
OOPS!! I meant to squash this into patch 10 before posting. If you want to just review it separately I can squash it in before push. Or if you want to be pedantic I can squash it in and resend :-) On 6/24/20 11:34 PM, Laine Stump wrote: Signed-off-by: Laine Stump --- src/network

[PATCH 14/25] network: replace VIR_ALLOC/REALLOC with g_new0/g_renew

2020-06-24 Thread Laine Stump
most of these are long-lived or attached to some other object, but a couple are automatics, and can take advantage of g_autoptr. Signed-off-by: Laine Stump --- src/network/bridge_driver.c | 44 +++-- 1 file changed, 13 insertions(+), 31 deletions(-) diff --git

[PATCH 18/25] nwfilter: define a typedef for struct ebtablesSubChainInst

2020-06-24 Thread Laine Stump
Signed-off-by: Laine Stump --- src/nwfilter/nwfilter_ebiptables_driver.c | 14 -- 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/nwfilter/nwfilter_ebiptables_driver.c b/src/nwfilter/nwfilter_ebiptables_driver.c index 8b77578117..cc814235aa 100644 --- a/src

[PATCH 24/25] nwfilter: use standard label names when reasonable

2020-06-24 Thread Laine Stump
Rather than having labels named exit, done, exit_snooprequnlock, skip_rename, etc, use the standard "cleanup" label. And instead of err_exit, malformed, tear_down_tmpebchains, use "error". Signed-off-by: Laine Stump --- src/nwfilter/nwfilter_dhcpsnoop.c | 36

[PATCH 08/25] network: fix memory leak in networkBuildDhcpDaemonCommandLine()

2020-06-24 Thread Laine Stump
hostsfilestr was not being freed. This will be turned into g_autofree in an upcoming patch converting a lot more of the same file to using g_auto*, but I wanted to make a separate patch for this first so the other patch is simpler to review. Signed-off-by: Laine Stump --- src/network

[PATCH 11/25] network: use g_free() in place of remaining VIR_FREE()

2020-06-24 Thread Laine Stump
Signed-off-by: Laine Stump --- src/network/bridge_driver.c | 59 + 1 file changed, 33 insertions(+), 26 deletions(-) diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 668aa9ca88..a1b2f5b6c7 100644 --- a/src/network/bridge_driver.c

[PATCH 21/25] nwfilter: convert local pointers to use g_auto*

2020-06-24 Thread Laine Stump
Signed-off-by: Laine Stump --- src/nwfilter/nwfilter_dhcpsnoop.c | 91 src/nwfilter/nwfilter_ebiptables_driver.c | 170 +- src/nwfilter/nwfilter_gentech_driver.c| 19 +-- src/nwfilter/nwfilter_learnipaddr.c | 9 +- 4 files changed, 108

[PATCH 20/25] nwfilter: remove unnecessary code from ebtablesGetSubChainInsts()

2020-06-24 Thread Laine Stump
sense for the function that is creating the entire list to be the one freeing the entire list, rather than having a function whose purpose is only to create *one item* on the list freeing the entire list). Signed-off-by: Laine Stump --- src/nwfilter/nwfilter_ebiptables_driver.c | 6 -- 1 file

[PATCH 10/25] network: convert local pointers to g_auto*

2020-06-24 Thread Laine Stump
This includes those that use plain VIR_FREE() as well as those that have a cleanup function defined for use via g_auto/g_autoptr (virCommand, virFirewall, virBuffer, virJSONValue etc). Signed-off-by: Laine Stump --- src/network/bridge_driver.c | 477 +++--- src

[PATCH 12/25] network: make networkDnsmasqXmlNsDef private to bridge_driver.c

2020-06-24 Thread Laine Stump
This struct isn't used anywhere else. Signed-off-by: Laine Stump --- src/network/bridge_driver.c | 10 ++ src/network/bridge_driver.h | 9 - 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index

[PATCH 16/25] squash into 'network: convert local pointers to g_auto*'

2020-06-24 Thread Laine Stump
Signed-off-by: Laine Stump --- src/network/bridge_driver_linux.c | 9 ++--- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/src/network/bridge_driver_linux.c b/src/network/bridge_driver_linux.c index 0d0ac730f2..7f765bcf99 100644 --- a/src/network/bridge_driver_linux.c +++ b

[PATCH 04/25] util: validate return from xmlNodeGetContent before use

2020-06-24 Thread Laine Stump
indicate a missing value in the document, so we don't report an OOM error, but just don't try to use it for, e.g., conversion to an integer. Signed-off-by: Laine Stump --- src/conf/domain_conf.c | 28 ++-- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/src

[PATCH 22/25] nwfilter: convert remaining VIR_FREE() to g_free()

2020-06-24 Thread Laine Stump
Signed-off-by: Laine Stump --- src/nwfilter/nwfilter_dhcpsnoop.c | 16 src/nwfilter/nwfilter_driver.c| 10 +- src/nwfilter/nwfilter_ebiptables_driver.c | 2 +- src/nwfilter/nwfilter_gentech_driver.c| 6 +++--- src/nwfilter/nwfilter_learnipaddr.c

[PATCH 00/25] Several g_auto* conversion and related small bugfixes

2020-06-24 Thread Laine Stump
g_auto*, g_new0, and g_free, but there is also a some of elimination of labels, fixing actual or theoretical memory leaks, modifications for style, etc. None of it should have any functional effect (except the fixing one or two memory leaks). Laine Stump (25): conf, vmx: check for OOM after calling

[PATCH 13/25] define g_autoptr cleanup function for virNetworkDHCPLease

2020-06-24 Thread Laine Stump
virNetworkDHCPLease and virNetworkDHCPLeaseFree() are declared in the public API file libvirt-network.h, and we can't pollute that with glib macro invocations, so put this in src/datatypes.h next to the other virNetwork items. Signed-off-by: Laine Stump --- src/datatypes.h | 2 ++ 1 file

[PATCH 02/25] use g_autoptr for all xmlBuffers

2020-06-24 Thread Laine Stump
in virxml.h) Signed-off-by: Laine Stump --- src/conf/domain_conf.c | 4 +--- src/conf/network_conf.c | 4 +--- src/util/virxml.c | 12 +++- src/vmx/vmx.c | 10 +++--- 4 files changed, 8 insertions(+), 22 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf

[PATCH 09/25] util: add g_autoptr cleanup function for virFirewall objects

2020-06-24 Thread Laine Stump
Put in a separate patch so that two future patches can be re-ordered / selectively backported independent of each other. Signed-off-by: Laine Stump --- src/util/virfirewall.h | 1 + 1 file changed, 1 insertion(+) diff --git a/src/util/virfirewall.h b/src/util/virfirewall.h index 6148f46827

[PATCH 01/25] conf, vmx: check for OOM after calling xmlBufferCreate()

2020-06-24 Thread Laine Stump
code end up in libvirt.so, which is linked and called directly by applications that may themselves use libxml2 (and may have already set their own alternate malloc()), e.g. drivers like esx which live totally in the library rather than a separate process.) Signed-off-by: Laine Stump --- src/conf

<    4   5   6   7   8   9   10   11   12   13   >