On 2/1/22 17:59, Erik Skultety wrote:
On Tue, Feb 01, 2022 at 05:23:52PM +0100, Michal Prívozník wrote:
On 2/1/22 17:18, Erik Skultety wrote:
On Mon, Jan 31, 2022 at 03:53:42PM +0100, Michal Privoznik wrote:
There are few places where the g_steal_pointer() is open coded. Switch them to calling the g_steal_pointer() function instead. Generated by the following spatch:
@ rule1 @ expression a, b; @@ <... - b = a; ... when != b - a = NULL; + b = g_steal_pointer(&a); ...>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- ...
diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c index 7b684e04ba..52851ac507 100644 --- a/src/hyperv/hyperv_driver.c +++ b/src/hyperv/hyperv_driver.c @@ -1780,8 +1780,7 @@ hypervConnectOpen(virConnectPtr conn, virConnectAuthPtr auth,
priv->version = g_strdup(os->data->Version);
- conn->privateData = priv; - priv = NULL; + conn->privateData = g_steal_pointer(&priv); result = VIR_DRV_OPEN_SUCCESS;
cleanup: @@ -3518,9 +3517,8 @@ hypervConnectListAllDomains(virConnectPtr conn, doms[count++] = domain; }
- if (doms) - *domains = doms; - doms = NULL; + if (domains) + *domains = g_steal_pointer(&doms);
^this is not semantically identical, you need to fix it manually before pushing
Reviewed-by: Erik Skultety <eskultet@redhat.com>
Yeah, hyperv code doesn't get as much love as other drivers, but basically, @domains is allocated iff @doms != NULL, there's the following code earlier in the function:
if (domains) { doms = g_new0(virDomainPtr, 1); ndoms = 1; }
I find the new version easier to read, since @domains is an argument of the function. Do you want me to post a separate patch that does
Well, it's not consistent with the rest which is not good for commit history in case this introduced a bug for some reason...
It's not, but I think it's trivial enough to be contained in this commit. But alright, let me keep the original condition and push.
s/doms/domains/?
It would probably need to a tiny bit more than that, e.g.:
diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c index 7b684e04ba..af71eef7e2 100644 --- a/src/hyperv/hyperv_driver.c +++ b/src/hyperv/hyperv_driver.c @@ -3423,7 +3423,6 @@ hypervConnectListAllDomains(virConnectPtr conn, g_autoptr(Msvm_ComputerSystem) computerSystemList = NULL; Msvm_ComputerSystem *computerSystem = NULL; size_t ndoms; - virDomainPtr domain; virDomainPtr *doms = NULL; int count = 0; int ret = -1; @@ -3475,6 +3474,7 @@ hypervConnectListAllDomains(virConnectPtr conn,
for (computerSystem = computerSystemList; computerSystem != NULL; computerSystem = computerSystem->next) { + virDomainPtr domain = NULL;
/* filter by domain state */ if (MATCH(VIR_CONNECT_LIST_DOMAINS_FILTERS_STATE)) { @@ -3509,13 +3509,11 @@ hypervConnectListAllDomains(virConnectPtr conn,
VIR_RESIZE_N(doms, ndoms, count, 2);
- domain = NULL; - if (hypervMsvmComputerSystemToDomain(conn, computerSystem, &domain) < 0) goto cleanup;
- doms[count++] = domain; + doms[count++] = g_steal_pointer(domain); }
if (doms)
This is unrelated. Worth pushing, but unrelated. Note, there are three variables: doms, domain, and domains. My patch touches only the first and the last one. Michal