[PATCH v2] Remove VIR_FREE in favor of g_autofree in some functions in libvrit-domain.c

Mostafa posted 1 patch 1 month, 2 weeks ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/AM9P193MB1126F0D4A68A817E88A67107BB242@AM9P193MB1126.EURP193.PROD.OUTLOOK.COM
src/libvirt-domain.c | 32 ++++++++------------------------
1 file changed, 8 insertions(+), 24 deletions(-)
[PATCH v2] Remove VIR_FREE in favor of g_autofree in some functions in libvrit-domain.c
Posted by Mostafa 1 month, 2 weeks ago
Signed-off-by: Mostafa <recenum@outlook.com>
---
 src/libvirt-domain.c | 32 ++++++++------------------------
 1 file changed, 8 insertions(+), 24 deletions(-)

diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index 83abad251e..4ba3563c9e 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -884,7 +884,7 @@ virDomainSave(virDomainPtr domain, const char *to)
 
     if (conn->driver->domainSave) {
         int ret;
-        char *absolute_to;
+        g_autofree char *absolute_to = NULL;
 
         /* We must absolutize the file path as the save is done out of process */
         if (!(absolute_to = g_canonicalize_filename(to, NULL))) {
@@ -895,8 +895,6 @@ virDomainSave(virDomainPtr domain, const char *to)
 
         ret = conn->driver->domainSave(domain, absolute_to);
 
-        VIR_FREE(absolute_to);
-
         if (ret < 0)
             goto error;
         return ret;
@@ -974,7 +972,7 @@ virDomainSaveFlags(virDomainPtr domain, const char *to,
 
     if (conn->driver->domainSaveFlags) {
         int ret;
-        char *absolute_to;
+        g_autofree char *absolute_to = NULL;
 
         /* We must absolutize the file path as the save is done out of process */
         if (!(absolute_to = g_canonicalize_filename(to, NULL))) {
@@ -985,8 +983,6 @@ virDomainSaveFlags(virDomainPtr domain, const char *to,
 
         ret = conn->driver->domainSaveFlags(domain, absolute_to, dxml, flags);
 
-        VIR_FREE(absolute_to);
-
         if (ret < 0)
             goto error;
         return ret;
@@ -1076,7 +1072,7 @@ virDomainRestore(virConnectPtr conn, const char *from)
 
     if (conn->driver->domainRestore) {
         int ret;
-        char *absolute_from;
+        g_autofree char *absolute_from = NULL;
 
         /* We must absolutize the file path as the restore is done out of process */
         if (!(absolute_from = g_canonicalize_filename(from, NULL))) {
@@ -1087,8 +1083,6 @@ virDomainRestore(virConnectPtr conn, const char *from)
 
         ret = conn->driver->domainRestore(conn, absolute_from);
 
-        VIR_FREE(absolute_from);
-
         if (ret < 0)
             goto error;
         return ret;
@@ -1156,7 +1150,7 @@ virDomainRestoreFlags(virConnectPtr conn, const char *from, const char *dxml,
 
     if (conn->driver->domainRestoreFlags) {
         int ret;
-        char *absolute_from;
+        g_autofree char *absolute_from = NULL;
 
         /* We must absolutize the file path as the restore is done out of process */
         if (!(absolute_from = g_canonicalize_filename(from, NULL))) {
@@ -1168,8 +1162,6 @@ virDomainRestoreFlags(virConnectPtr conn, const char *from, const char *dxml,
         ret = conn->driver->domainRestoreFlags(conn, absolute_from, dxml,
                                                flags);
 
-        VIR_FREE(absolute_from);
-
         if (ret < 0)
             goto error;
         return ret;
@@ -1263,7 +1255,7 @@ virDomainSaveImageGetXMLDesc(virConnectPtr conn, const char *file,
 
     if (conn->driver->domainSaveImageGetXMLDesc) {
         char *ret;
-        char *absolute_file;
+        g_autofree char *absolute_file = NULL;
 
         /* We must absolutize the file path as the read is done out of process */
         if (!(absolute_file = g_canonicalize_filename(file, NULL))) {
@@ -1275,8 +1267,6 @@ virDomainSaveImageGetXMLDesc(virConnectPtr conn, const char *file,
         ret = conn->driver->domainSaveImageGetXMLDesc(conn, absolute_file,
                                                       flags);
 
-        VIR_FREE(absolute_file);
-
         if (!ret)
             goto error;
         return ret;
@@ -1338,7 +1328,7 @@ virDomainSaveImageDefineXML(virConnectPtr conn, const char *file,
 
     if (conn->driver->domainSaveImageDefineXML) {
         int ret;
-        char *absolute_file;
+        g_autofree char *absolute_file = NULL;
 
         /* We must absolutize the file path as the read is done out of process */
         if (!(absolute_file = g_canonicalize_filename(file, NULL))) {
@@ -1350,8 +1340,6 @@ virDomainSaveImageDefineXML(virConnectPtr conn, const char *file,
         ret = conn->driver->domainSaveImageDefineXML(conn, absolute_file,
                                                      dxml, flags);
 
-        VIR_FREE(absolute_file);
-
         if (ret < 0)
             goto error;
         return ret;
@@ -1415,7 +1403,7 @@ virDomainCoreDump(virDomainPtr domain, const char *to, unsigned int flags)
 
     if (conn->driver->domainCoreDump) {
         int ret;
-        char *absolute_to;
+        g_autofree char *absolute_to = NULL;
 
         /* We must absolutize the file path as the save is done out of process */
         if (!(absolute_to = g_canonicalize_filename(to, NULL))) {
@@ -1426,8 +1414,6 @@ virDomainCoreDump(virDomainPtr domain, const char *to, unsigned int flags)
 
         ret = conn->driver->domainCoreDump(domain, absolute_to, flags);
 
-        VIR_FREE(absolute_to);
-
         if (ret < 0)
             goto error;
         return ret;
@@ -1501,7 +1487,7 @@ virDomainCoreDumpWithFormat(virDomainPtr domain, const char *to,
 
     if (conn->driver->domainCoreDumpWithFormat) {
         int ret;
-        char *absolute_to;
+        g_autofree char *absolute_to = NULL;
 
         /* We must absolutize the file path as the save is done out of process */
         if (!(absolute_to = g_canonicalize_filename(to, NULL))) {
@@ -1513,8 +1499,6 @@ virDomainCoreDumpWithFormat(virDomainPtr domain, const char *to,
         ret = conn->driver->domainCoreDumpWithFormat(domain, absolute_to,
                                                      dumpformat, flags);
 
-        VIR_FREE(absolute_to);
-
         if (ret < 0)
             goto error;
         return ret;
-- 
2.34.1
_______________________________________________
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-leave@lists.libvirt.org
Re: [PATCH v2] Remove VIR_FREE in favor of g_autofree in some functions in libvrit-domain.c
Posted by Peter Krempa 1 month, 2 weeks ago
On Tue, Mar 12, 2024 at 00:11:28 +0200, Mostafa wrote:

The patch looks good now, but you've used multiple names in various
fields over the two postings and the preference in terms of both
authorship and sign-off is to use a full real name

Used previously:

> From: مصطفي محمود كمال الدين <48567303+moste00@users.noreply.github.com>
> From: m kamal <recenum@outlook.com>

The one in this patch:

> Signed-off-by: Mostafa <recenum@outlook.com>

I don't want to assume anything about foreign names, but the last one
looks more like a nickname or given name only.

> ---
>  src/libvirt-domain.c | 32 ++++++++------------------------
>  1 file changed, 8 insertions(+), 24 deletions(-)

For the code:

Reviewed-by: Peter Krempa <pkrempa@redhat.com>
_______________________________________________
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-leave@lists.libvirt.org
Re: [PATCH v2] Remove VIR_FREE in favor of g_autofree in some functions in libvrit-domain.c
Posted by m kamal 1 month, 2 weeks ago
Thanks a lot for the review, Peter.

The first name I used was by mistake, it's the name of my Github profile, Github desktop probably auto-configured it as the global user name for git on my machine. It's in Arabic so it could confuse many non-Unicode-aware programs or protocols. The other name "m kamal" is just a contraction I wrote when I was subscribing to the mailing list from the web UI, I didn't realize it's important.

I will change my configured name in the .gitpublish config to be the longer name "Mostafa Mahmoud", that's the first 2 names of my legal name, it appears that most names here are also 2 names. If there is also a way to change my forum name "m kamal" please let me know.

Sorry for any inconvenience. One last question: After the patch got your approval, is there something on my end to do so that it gets merged? Will it automatically gets picked up by the Gitlab CI pipeline somehow?

Cheers,
Mostafa Mahmoud.
________________________________
From: Peter Krempa <pkrempa@redhat.com>
Sent: Tuesday, March 12, 2024 9:57 AM
To: Mostafa <recenum@outlook.com>
Cc: devel@lists.libvirt.org <devel@lists.libvirt.org>
Subject: Re: [PATCH v2] Remove VIR_FREE in favor of g_autofree in some functions in libvrit-domain.c

On Tue, Mar 12, 2024 at 00:11:28 +0200, Mostafa wrote:

The patch looks good now, but you've used multiple names in various
fields over the two postings and the preference in terms of both
authorship and sign-off is to use a full real name

Used previously:

> From: مصطفي محمود كمال الدين <48567303+moste00@users.noreply.github.com>
> From: m kamal <recenum@outlook.com>

The one in this patch:

> Signed-off-by: Mostafa <recenum@outlook.com>

I don't want to assume anything about foreign names, but the last one
looks more like a nickname or given name only.

> ---
>  src/libvirt-domain.c | 32 ++++++++------------------------
>  1 file changed, 8 insertions(+), 24 deletions(-)

For the code:

Reviewed-by: Peter Krempa <pkrempa@redhat.com>

_______________________________________________
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-leave@lists.libvirt.org
Re: [PATCH v2] Remove VIR_FREE in favor of g_autofree in some functions in libvrit-domain.c
Posted by Peter Krempa 1 month, 1 week ago
On Tue, Mar 12, 2024 at 17:58:08 +0000, m kamal wrote:
> Thanks a lot for the review, Peter.
> 
> The first name I used was by mistake, it's the name of my Github profile, Github desktop probably auto-configured it as the global user name for git on my machine. It's in Arabic so it could confuse many non-Unicode-aware programs or protocols. The other name "m kamal" is just a contraction I wrote when I was subscribing to the mailing list from the web UI, I didn't realize it's important.


Technically that is not important as it can be overriden by using a From
line similarly to your first subscription. The only important bit is
what will be recorded in the GIT history, you're free to use any other
version in other communication. 

> 
> I will change my configured name in the .gitpublish config to be the longer name "Mostafa Mahmoud", that's the first 2 names of my legal name, it appears that most names here are also 2 names. If there is also a way to change my forum name "m kamal" please let me know.a

If by 'forum' you mean the mailing list you should be able to change
that in https://lists.libvirt.org/user-profile/ once you log in.

> 
> Sorry for any inconvenience. One last question: After the patch got your approval, is there something on my end to do so that it gets merged? Will it automatically gets picked up by the Gitlab CI pipeline somehow?

Depending on whether you want to test if you configured your environment
propely you can either send a v3 with updated autorship and sign-off, or
just reply with the full format you're intending to use and I can modify
this patch and push it.

Pushing is done by one of the maintainers, which would be me since I've
reviewed it.
_______________________________________________
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-leave@lists.libvirt.org
Re: [PATCH v2] Remove VIR_FREE in favor of g_autofree in some functions in libvrit-domain.c
Posted by m kamal 1 month, 1 week ago
Please advise on what actions (if any) should be  taken so that the changes in the patch is merged into main.

Cheers,
Mostafa Mahmoud.
________________________________
From: m kamal <recenum@outlook.com>
Sent: Tuesday, March 12, 2024 5:58 PM
To: Peter Krempa <pkrempa@redhat.com>
Cc: devel@lists.libvirt.org <devel@lists.libvirt.org>
Subject: Re: [PATCH v2] Remove VIR_FREE in favor of g_autofree in some functions in libvrit-domain.c

Thanks a lot for the review, Peter.

The first name I used was by mistake, it's the name of my Github profile, Github desktop probably auto-configured it as the global user name for git on my machine. It's in Arabic so it could confuse many non-Unicode-aware programs or protocols. The other name "m kamal" is just a contraction I wrote when I was subscribing to the mailing list from the web UI, I didn't realize it's important.

I will change my configured name in the .gitpublish config to be the longer name "Mostafa Mahmoud", that's the first 2 names of my legal name, it appears that most names here are also 2 names. If there is also a way to change my forum name "m kamal" please let me know.

Sorry for any inconvenience. One last question: After the patch got your approval, is there something on my end to do so that it gets merged? Will it automatically gets picked up by the Gitlab CI pipeline somehow?

Cheers,
Mostafa Mahmoud.
________________________________
From: Peter Krempa <pkrempa@redhat.com>
Sent: Tuesday, March 12, 2024 9:57 AM
To: Mostafa <recenum@outlook.com>
Cc: devel@lists.libvirt.org <devel@lists.libvirt.org>
Subject: Re: [PATCH v2] Remove VIR_FREE in favor of g_autofree in some functions in libvrit-domain.c

On Tue, Mar 12, 2024 at 00:11:28 +0200, Mostafa wrote:

The patch looks good now, but you've used multiple names in various
fields over the two postings and the preference in terms of both
authorship and sign-off is to use a full real name

Used previously:

> From: مصطفي محمود كمال الدين <48567303+moste00@users.noreply.github.com>
> From: m kamal <recenum@outlook.com>

The one in this patch:

> Signed-off-by: Mostafa <recenum@outlook.com>

I don't want to assume anything about foreign names, but the last one
looks more like a nickname or given name only.

> ---
>  src/libvirt-domain.c | 32 ++++++++------------------------
>  1 file changed, 8 insertions(+), 24 deletions(-)

For the code:

Reviewed-by: Peter Krempa <pkrempa@redhat.com>

_______________________________________________
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-leave@lists.libvirt.org