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@xxxxxxxxxx>
> >> ---
> > ...
> >
> >> 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@xxxxxxxxxx>
> >
>
> 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...
> 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)
Erik
