4:04 a.m.
After adopting g_get_*_dir(), internally, when implementing
virGetUser*Dir(), the libvirt functions changed their behaviour as NULL
is never ever returned by the GLib functions.
Knowing that, let's cleanup the callers' code of those functions,
removing the unnecessary checks.
While doing the cleanup mentioned above, I've noticed that some other
cleanups could be done, when touching the very same function, as using
g_autofree. It's done as part of this series as well.
phyp driver was not touched as there's some plan to just drop its code
entirely: https://www.redhat.com/archives/libvir-list/2019-December/msg01162.html
Fabiano Fidêncio (42):
tools: Use g_autofree on cmdCd()
vbox: Don't leak virGetUserDirectory()'s output
rpc: Use g_autofree on virNetClientNewLibSSH2()
rpc: Use g_autofree on virNetClientNewLibssh()
rpc: Don't check the output of virGetUserDirectory()
qemu: Don't check the output of virGetUserDirectory()
util: Don't check the output of virGetUserConfigDirectory()
storage: Don't check the output of virGetUserConfigDirectory()
secret: Don't check the output of virGetUserConfigDirectory()
tools: Don't check the output of virGetUserCacheDirectory()
vbox: Use g_autofree on vboxDomainScreenshot()
vbox: Don't check the output of virGetUserCacheDirectory()
util: Use g_autofree on virLogSetDefaultOutputToFile()
util: Don't check the output of virGetUserCacheDirectory()
qemu: Don't check the output of virGetUserCacheDirectory()
rpc: Don't check the output of virGetUserConfigDirectory()
remote: Don't check the output of virGetUserConfigDirectory()
qemu: Don't check the output of virGetUserConfigDirectory()
network: Don't check the output of virGetUserConfigDirectory()
logging: Use g_autofree on virLogDaemonConfigFilePath()
logging: Don't check the output of virGetUserConfigDirectory()
locking: Use g_autofree on virLockDaemonConfigFilePath()
locking: Don't check the output of virGetUserConfigDirectory()
util: Don't check the output of virGetUserRuntimeDirectory()
storage: Don't check the output of virGetUserRuntimeDirectory()
secret: Don't check the output of virGetUserRuntimeDirectory()
rpc: Don't check the output of virGetUserRuntimeDirectory()
remote: Don't check the output of virGetUserRuntimeDirectory()
qemu: Don't check the output of virGetUserRuntimeDirectory()
node_device: Don't check the output of virGetUserRuntimeDirectory()
network: Don't check the output of virGetUserRuntimeDirectory()
logging: Use g_autofree on virLogManagerDaemonPath()
logging: Use g_autofree on virLogDaemonUnixSocketPaths()
logging: Use g_autofree on virLogDaemonExecRestartStatePath()
logging: Don't check the output of virGetUserRuntimeDirectory()
locking: Use g_autofree on virLockManagerLockDaemonPath()
locking: Use g_autofree on virLockDaemonUnixSocketPaths()
locking: Use g_autofree on virLockDaemonExecRestartStatePath()
locking: Don't check the output of virGetUserRuntimeDirectory()
interface: Don't check the output of virGetUserRuntimeDirectory()
admin: Use g_autofree on getSocketPath()
admin: Don't check the output of virGetUserRuntimeDirectory()
src/admin/libvirt-admin.c | 6 +--
src/interface/interface_backend_netcf.c | 3 +-
src/interface/interface_backend_udev.c | 3 +-
src/locking/lock_daemon.c | 31 +++----------
src/locking/lock_daemon_config.c | 9 +---
src/locking/lock_driver_lockd.c | 7 +--
src/logging/log_daemon.c | 31 +++----------
src/logging/log_daemon_config.c | 9 +---
src/logging/log_manager.c | 7 +--
src/network/bridge_driver.c | 2 -
src/node_device/node_device_hal.c | 3 +-
src/node_device/node_device_udev.c | 3 +-
src/qemu/qemu_conf.c | 7 +--
src/qemu/qemu_interop_config.c | 3 --
src/remote/remote_daemon.c | 8 +---
src/remote/remote_daemon_config.c | 6 +--
src/remote/remote_driver.c | 3 +-
src/rpc/virnetclient.c | 59 +++++++++----------------
src/rpc/virnetsocket.c | 3 +-
src/rpc/virnettlscontext.c | 12 -----
src/secret/secret_driver.c | 6 +--
src/storage/storage_driver.c | 2 -
src/util/virauth.c | 3 +-
src/util/virconf.c | 2 -
src/util/virhostdev.c | 3 +-
src/util/virlog.c | 13 ++----
src/util/virpidfile.c | 3 +-
src/vbox/vbox_common.c | 12 ++---
src/vbox/vbox_storage.c | 7 ++-
tools/vsh.c | 13 ++----
30 files changed, 73 insertions(+), 206 deletions(-)
--
2.24.1
4:04 a.m.
New subject: [libvirt] [PATCH 01/42] tools: Use g_autofree on cmdCd()
Signed-off-by: Fabiano Fidêncio <fidencio(a)redhat.com>
---
tools/vsh.c | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/tools/vsh.c b/tools/vsh.c
index 5ccda5ab44..bbb6227130 100644
--- a/tools/vsh.c
+++ b/tools/vsh.c
@@ -3175,8 +3175,7 @@ bool
cmdCd(vshControl *ctl, const vshCmd *cmd)
{
const char *dir = NULL;
- char *dir_malloced = NULL;
- bool ret = true;
+ g_autofree char *dir_malloced = NULL;
char ebuf[1024];
if (!ctl->imode) {
@@ -3192,11 +3191,10 @@ cmdCd(vshControl *ctl, const vshCmd *cmd)
if (chdir(dir) == -1) {
vshError(ctl, _("cd: %s: %s"),
virStrerror(errno, ebuf, sizeof(ebuf)), dir);
- ret = false;
+ return false;
}
- VIR_FREE(dir_malloced);
- return ret;
+ return true;
}
const vshCmdOptDef opts_echo[] = {
--
2.24.1
10:22 a.m.
New subject: [libvirt] [PATCH 01/42] tools: Use g_autofree on cmdCd()
In the commit summary:
s/on/in/
On Thu, Dec 19, 2019 at 11:04:06AM +0100, Fabiano Fidêncio wrote:
Signed-off-by: Fabiano Fidêncio <fidencio(a)redhat.com>
---
tools/vsh.c | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
4:04 a.m.
New subject: [libvirt] [PATCH 02/42] vbox: Don't leak virGetUserDirectory()'s output
On vboxStorageVolCreateXML(), virGetUserDirectory() was called without
freeing its content later on.
Signed-off-by: Fabiano Fidêncio <fidencio(a)redhat.com>
---
src/vbox/vbox_storage.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/src/vbox/vbox_storage.c b/src/vbox/vbox_storage.c
index 790b0e4997..7f240c5333 100644
--- a/src/vbox/vbox_storage.c
+++ b/src/vbox/vbox_storage.c
@@ -410,6 +410,7 @@ vboxStorageVolCreateXML(virStoragePoolPtr pool,
resultCodeUnion resultCode;
virStorageVolPtr ret = NULL;
g_autoptr(virStorageVolDef) def = NULL;
+ g_autofree char *homedir = NULL;
if (!data->vboxObj)
return ret;
@@ -442,8 +443,10 @@ vboxStorageVolCreateXML(virStoragePoolPtr pool,
}
/* If target.path isn't given, use default path ~/.VirtualBox/image_name */
- if (!def->target.path)
- def->target.path = g_strdup_printf("%s/.VirtualBox/%s",
virGetUserDirectory(), def->name);
+ if (!def->target.path) {
+ homedir = virGetUserDirectory();
+ def->target.path = g_strdup_printf("%s/.VirtualBox/%s", homedir,
def->name);
+ }
VBOX_UTF8_TO_UTF16(def->target.path, &hddNameUtf16);
if (!hddFormatUtf16 || !hddNameUtf16)
--
2.24.1
10:25 a.m.
New subject: [libvirt] [PATCH 02/42] vbox: Don't leak virGetUserDirectory()'s output
On Thu, Dec 19, 2019 at 11:04:07AM +0100, Fabiano Fidêncio wrote:
On vboxStorageVolCreateXML(), virGetUserDirectory() was called without
s/On/In/
freeing its content later on.
Signed-off-by: Fabiano Fidêncio <fidencio(a)redhat.com>
---
src/vbox/vbox_storage.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
4:04 a.m.
New subject: [libvirt] [PATCH 03/42] rpc: Use g_autofree on virNetClientNewLibSSH2()
Signed-off-by: Fabiano Fidêncio <fidencio(a)redhat.com>
---
src/rpc/virnetclient.c | 18 ++++++------------
1 file changed, 6 insertions(+), 12 deletions(-)
diff --git a/src/rpc/virnetclient.c b/src/rpc/virnetclient.c
index 47a17d30f7..75e653fec8 100644
--- a/src/rpc/virnetclient.c
+++ b/src/rpc/virnetclient.c
@@ -442,13 +442,13 @@ virNetClientPtr virNetClientNewLibSSH2(const char *host,
virNetClientPtr ret = NULL;
virBuffer buf = VIR_BUFFER_INITIALIZER;
- char *nc = NULL;
- char *command = NULL;
+ g_autofree char *nc = NULL;
+ g_autofree char *command = NULL;
- char *homedir = NULL;
- char *confdir = NULL;
- char *knownhosts = NULL;
- char *privkey = NULL;
+ g_autofree char *homedir = NULL;
+ g_autofree char *confdir = NULL;
+ g_autofree char *knownhosts = NULL;
+ g_autofree char *privkey = NULL;
/* Use default paths for known hosts an public keys if not provided */
if (knownHostsPath) {
@@ -517,12 +517,6 @@ virNetClientPtr virNetClientNewLibSSH2(const char *host,
goto cleanup;
cleanup:
- VIR_FREE(command);
- VIR_FREE(privkey);
- VIR_FREE(knownhosts);
- VIR_FREE(homedir);
- VIR_FREE(confdir);
- VIR_FREE(nc);
return ret;
no_memory:
--
2.24.1
10:35 a.m.
New subject: [libvirt] [PATCH 03/42] rpc: Use g_autofree on virNetClientNewLibSSH2()
s/on/in/
On Thu, Dec 19, 2019 at 11:04:08AM +0100, Fabiano Fidêncio wrote:
Signed-off-by: Fabiano Fidêncio <fidencio(a)redhat.com>
---
src/rpc/virnetclient.c | 18 ++++++------------
1 file changed, 6 insertions(+), 12 deletions(-)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
1 p.m.
New subject: [libvirt] [PATCH 03/42] rpc: Use g_autofree on virNetClientNewLibSSH2()
On 12/19/19 7:04 AM, Fabiano Fidêncio wrote:
Signed-off-by: Fabiano Fidêncio <fidencio(a)redhat.com>
---
src/rpc/virnetclient.c | 18 ++++++------------
1 file changed, 6 insertions(+), 12 deletions(-)
[...]
if (knownHostsPath) {
@@ -517,12 +517,6 @@ virNetClientPtr virNetClientNewLibSSH2(const char *host,
goto cleanup;
cleanup:
- VIR_FREE(command);
- VIR_FREE(privkey);
- VIR_FREE(knownhosts);
- VIR_FREE(homedir);
- VIR_FREE(confdir);
- VIR_FREE(nc);
return ret;
This will leave a now unneeded 'cleanup' label:
cleanup:
return ret;
In a quick glance at the patch series I noticed that Patch 41 also leaves
an unused 'cleanup' label as well after the changes.
Thanks,
DHB
no_memory:
4:17 p.m.
New subject: [libvirt] [PATCH 03/42] rpc: Use g_autofree on virNetClientNewLibSSH2()
On Thu, Dec 19, 2019 at 8:00 PM Daniel Henrique Barboza
<danielhb413(a)gmail.com> wrote:
On 12/19/19 7:04 AM, Fabiano Fidêncio wrote:
> Signed-off-by: Fabiano Fidêncio <fidencio(a)redhat.com>
> ---
> src/rpc/virnetclient.c | 18 ++++++------------
> 1 file changed, 6 insertions(+), 12 deletions(-)
>
[...]
> if (knownHostsPath) {
> @@ -517,12 +517,6 @@ virNetClientPtr virNetClientNewLibSSH2(const char *host,
> goto cleanup;
>
> cleanup:
> - VIR_FREE(command);
> - VIR_FREE(privkey);
> - VIR_FREE(knownhosts);
> - VIR_FREE(homedir);
> - VIR_FREE(confdir);
> - VIR_FREE(nc);
> return ret;
>
This will leave a now unneeded 'cleanup' label:
cleanup:
return ret;
In a quick glance at the patch series I noticed that Patch 41 also leaves
an unused 'cleanup' label as well after the changes.
Daniel,
Please, take a look at the no_memory label. It calls the cleanup one.
Best Regards,
--
Fabiano Fidêncio
5:30 p.m.
New subject: [libvirt] [PATCH 03/42] rpc: Use g_autofree on virNetClientNewLibSSH2()
On 12/19/19 7:17 PM, Fabiano Fidêncio wrote:
On Thu, Dec 19, 2019 at 8:00 PM Daniel Henrique Barboza
<danielhb413(a)gmail.com> wrote:
>
>
>
> On 12/19/19 7:04 AM, Fabiano Fidêncio wrote:
>> Signed-off-by: Fabiano Fidêncio <fidencio(a)redhat.com>
>> ---
>> src/rpc/virnetclient.c | 18 ++++++------------
>> 1 file changed, 6 insertions(+), 12 deletions(-)
>>
> [...]
>
>> if (knownHostsPath) {
>> @@ -517,12 +517,6 @@ virNetClientPtr virNetClientNewLibSSH2(const char *host,
>> goto cleanup;
>>
>> cleanup:
>> - VIR_FREE(command);
>> - VIR_FREE(privkey);
>> - VIR_FREE(knownhosts);
>> - VIR_FREE(homedir);
>> - VIR_FREE(confdir);
>> - VIR_FREE(nc);
>> return ret;
>>
>
> This will leave a now unneeded 'cleanup' label:
>
> cleanup:
> return ret;
>
>
> In a quick glance at the patch series I noticed that Patch 41 also leaves
> an unused 'cleanup' label as well after the changes.
Daniel,
Please, take a look at the no_memory label. It calls the cleanup one.
I am afraid I wasn't clear. By 'unneeded' I mean a label that is a simply
'return'
call. 'cleanup' is doing no cleanups anymore. So this this guy:
no_memory:
virReportOOMError();
goto cleanup;
turns into:
no_memory:
virReportOOMError();
return ret;
And in fact, since ret is initialized with NULL, and it will never be set before
a no_memory jump, you can even do:
no_memory:
virReportOOMError();
return NULL;
All 'cleanup' jumps can be changed to 'return NULL' and the line that sets
'ret'
can turn into
return virNetClientNew(sock, NULL));
And you can get rid of both the 'cleanup' label and the 'ret' variable.
All of this is just me being pedantic though :) I saw that you erased some
'exit' labels in other patches and wanted to point out that this label is
also removable. In the end I believe you can keep this patch just adding
g_autofree and removing VIR_FREE calls and, if you want, remove these labels
in a separated patch.
Thanks,
DHB
Best Regards,
5:34 p.m.
New subject: [libvirt] [PATCH 03/42] rpc: Use g_autofree on virNetClientNewLibSSH2()
On Fri, Dec 20, 2019 at 12:30 AM Daniel Henrique Barboza
<danielhb413(a)gmail.com> wrote:
On 12/19/19 7:17 PM, Fabiano Fidêncio wrote:
> On Thu, Dec 19, 2019 at 8:00 PM Daniel Henrique Barboza
> <danielhb413(a)gmail.com> wrote:
>>
>>
>>
>> On 12/19/19 7:04 AM, Fabiano Fidêncio wrote:
>>> Signed-off-by: Fabiano Fidêncio <fidencio(a)redhat.com>
>>> ---
>>> src/rpc/virnetclient.c | 18 ++++++------------
>>> 1 file changed, 6 insertions(+), 12 deletions(-)
>>>
>> [...]
>>
>>> if (knownHostsPath) {
>>> @@ -517,12 +517,6 @@ virNetClientPtr virNetClientNewLibSSH2(const char
*host,
>>> goto cleanup;
>>>
>>> cleanup:
>>> - VIR_FREE(command);
>>> - VIR_FREE(privkey);
>>> - VIR_FREE(knownhosts);
>>> - VIR_FREE(homedir);
>>> - VIR_FREE(confdir);
>>> - VIR_FREE(nc);
>>> return ret;
>>>
>>
>> This will leave a now unneeded 'cleanup' label:
>>
>> cleanup:
>> return ret;
>>
>>
>> In a quick glance at the patch series I noticed that Patch 41 also leaves
>> an unused 'cleanup' label as well after the changes.
>
> Daniel,
>
> Please, take a look at the no_memory label. It calls the cleanup one.
I am afraid I wasn't clear. By 'unneeded' I mean a label that is a simply
'return'
call. 'cleanup' is doing no cleanups anymore. So this this guy:
no_memory:
virReportOOMError();
goto cleanup;
turns into:
no_memory:
virReportOOMError();
return ret;
And in fact, since ret is initialized with NULL, and it will never be set before
a no_memory jump, you can even do:
no_memory:
virReportOOMError();
return NULL;
All 'cleanup' jumps can be changed to 'return NULL' and the line that
sets 'ret'
can turn into
return virNetClientNew(sock, NULL));
And you can get rid of both the 'cleanup' label and the 'ret' variable.
All of this is just me being pedantic though :) I saw that you erased some
'exit' labels in other patches and wanted to point out that this label is
also removable. In the end I believe you can keep this patch just adding
g_autofree and removing VIR_FREE calls and, if you want, remove these labels
in a separated patch.
I see. I'll approach this in a different series, tho.
Best Regards,
--
Fabiano Fidêncio
4:04 a.m.
New subject: [libvirt] [PATCH 04/42] rpc: Use g_autofree on virNetClientNewLibssh()
Signed-off-by: Fabiano Fidêncio <fidencio(a)redhat.com>
---
src/rpc/virnetclient.c | 18 ++++++------------
1 file changed, 6 insertions(+), 12 deletions(-)
diff --git a/src/rpc/virnetclient.c b/src/rpc/virnetclient.c
index 75e653fec8..1b882a261a 100644
--- a/src/rpc/virnetclient.c
+++ b/src/rpc/virnetclient.c
@@ -545,13 +545,13 @@ virNetClientPtr virNetClientNewLibssh(const char *host,
virNetClientPtr ret = NULL;
virBuffer buf = VIR_BUFFER_INITIALIZER;
- char *nc = NULL;
- char *command = NULL;
+ g_autofree char *nc = NULL;
+ g_autofree char *command = NULL;
- char *homedir = NULL;
- char *confdir = NULL;
- char *knownhosts = NULL;
- char *privkey = NULL;
+ g_autofree char *homedir = NULL;
+ g_autofree char *confdir = NULL;
+ g_autofree char *knownhosts = NULL;
+ g_autofree char *privkey = NULL;
/* Use default paths for known hosts an public keys if not provided */
if (knownHostsPath) {
@@ -609,12 +609,6 @@ virNetClientPtr virNetClientNewLibssh(const char *host,
goto cleanup;
cleanup:
- VIR_FREE(command);
- VIR_FREE(privkey);
- VIR_FREE(knownhosts);
- VIR_FREE(homedir);
- VIR_FREE(confdir);
- VIR_FREE(nc);
return ret;
no_memory:
--
2.24.1
10:36 a.m.
New subject: [libvirt] [PATCH 04/42] rpc: Use g_autofree on virNetClientNewLibssh()
s/on/in/
On Thu, Dec 19, 2019 at 11:04:09AM +0100, Fabiano Fidêncio wrote:
Signed-off-by: Fabiano Fidêncio <fidencio(a)redhat.com>
---
src/rpc/virnetclient.c | 18 ++++++------------
1 file changed, 6 insertions(+), 12 deletions(-)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
4:04 a.m.
New subject: [libvirt] [PATCH 05/42] rpc: Don't check the output of virGetUserDirectory()
virGetUserDirectory() *never* *ever* returns NULL, making the checks for
it completely unnecessary.
Signed-off-by: Fabiano Fidêncio <fidencio(a)redhat.com>
---
src/rpc/virnetclient.c | 12 ++++--------
src/rpc/virnettlscontext.c | 12 ------------
2 files changed, 4 insertions(+), 20 deletions(-)
diff --git a/src/rpc/virnetclient.c b/src/rpc/virnetclient.c
index 1b882a261a..40e5fa35e2 100644
--- a/src/rpc/virnetclient.c
+++ b/src/rpc/virnetclient.c
@@ -466,10 +466,8 @@ virNetClientPtr virNetClientNewLibSSH2(const char *host,
privkey = g_strdup(privkeyPath);
} else {
homedir = virGetUserDirectory();
- if (homedir) {
- if (virNetClientFindDefaultSshKey(homedir, &privkey) < 0)
- goto no_memory;
- }
+ if (virNetClientFindDefaultSshKey(homedir, &privkey) < 0)
+ goto no_memory;
}
if (!authMethods) {
@@ -566,10 +564,8 @@ virNetClientPtr virNetClientNewLibssh(const char *host,
privkey = g_strdup(privkeyPath);
} else {
homedir = virGetUserDirectory();
- if (homedir) {
- if (virNetClientFindDefaultSshKey(homedir, &privkey) < 0)
- goto no_memory;
- }
+ if (virNetClientFindDefaultSshKey(homedir, &privkey) < 0)
+ goto no_memory;
}
if (!authMethods) {
diff --git a/src/rpc/virnettlscontext.c b/src/rpc/virnettlscontext.c
index ec9dd35c46..08944f6771 100644
--- a/src/rpc/virnettlscontext.c
+++ b/src/rpc/virnettlscontext.c
@@ -805,9 +805,6 @@ static int virNetTLSContextLocateCredentials(const char *pkipath,
*/
userdir = virGetUserDirectory();
- if (!userdir)
- goto error;
-
user_pki_path = g_strdup_printf("%s/.pki/libvirt", userdir);
VIR_DEBUG("Trying to find TLS user credentials in %s", user_pki_path);
@@ -864,15 +861,6 @@ static int virNetTLSContextLocateCredentials(const char *pkipath,
VIR_FREE(userdir);
return 0;
-
- error:
- VIR_FREE(*cacert);
- VIR_FREE(*cacrl);
- VIR_FREE(*key);
- VIR_FREE(*cert);
- VIR_FREE(user_pki_path);
- VIR_FREE(userdir);
- return -1;
}
--
2.24.1
10 a.m.
New subject: [libvirt] [PATCH 05/42] rpc: Don't check the output of virGetUserDirectory()
On Thu, Dec 19, 2019 at 11:04:10AM +0100, Fabiano Fidêncio wrote:
virGetUserDirectory() *never* *ever* returns NULL, making the checks
for
it completely unnecessary.
Signed-off-by: Fabiano Fidêncio <fidencio(a)redhat.com>
---
src/rpc/virnetclient.c | 12 ++++--------
src/rpc/virnettlscontext.c | 12 ------------
2 files changed, 4 insertions(+), 20 deletions(-)
[...]
diff --git a/src/rpc/virnettlscontext.c b/src/rpc/virnettlscontext.c
index ec9dd35c46..08944f6771 100644
--- a/src/rpc/virnettlscontext.c
+++ b/src/rpc/virnettlscontext.c
@@ -805,9 +805,6 @@ static int virNetTLSContextLocateCredentials(const char *pkipath,
*/
userdir = virGetUserDirectory();
- if (!userdir)
- goto error;
-
user_pki_path = g_strdup_printf("%s/.pki/libvirt", userdir);
VIR_DEBUG("Trying to find TLS user credentials in %s",
user_pki_path);
@@ -864,15 +861,6 @@ static int virNetTLSContextLocateCredentials(const char *pkipath,
VIR_FREE(userdir);
return 0;
-
- error:
- VIR_FREE(*cacert);
- VIR_FREE(*cacrl);
- VIR_FREE(*key);
- VIR_FREE(*cert);
- VIR_FREE(user_pki_path);
- VIR_FREE(userdir);
- return -1;
This doesn't look right. Looks like some leftover from rebase where
you wanted to use g_autofree.
Pavel
10:21 a.m.
New subject: [libvirt] [PATCH 05/42] rpc: Don't check the output of virGetUserDirectory()
On Thu, Dec 19, 2019 at 5:07 PM Pavel Hrdina <phrdina(a)redhat.com> wrote:
On Thu, Dec 19, 2019 at 11:04:10AM +0100, Fabiano Fidêncio wrote:
> virGetUserDirectory() *never* *ever* returns NULL, making the checks for
> it completely unnecessary.
>
> Signed-off-by: Fabiano Fidêncio <fidencio(a)redhat.com>
> ---
> src/rpc/virnetclient.c | 12 ++++--------
> src/rpc/virnettlscontext.c | 12 ------------
> 2 files changed, 4 insertions(+), 20 deletions(-)
[...]
> diff --git a/src/rpc/virnettlscontext.c b/src/rpc/virnettlscontext.c
> index ec9dd35c46..08944f6771 100644
> --- a/src/rpc/virnettlscontext.c
> +++ b/src/rpc/virnettlscontext.c
> @@ -805,9 +805,6 @@ static int virNetTLSContextLocateCredentials(const char
*pkipath,
> */
> userdir = virGetUserDirectory();
>
> - if (!userdir)
> - goto error;
> -
> user_pki_path = g_strdup_printf("%s/.pki/libvirt", userdir);
>
> VIR_DEBUG("Trying to find TLS user credentials in %s",
user_pki_path);
> @@ -864,15 +861,6 @@ static int virNetTLSContextLocateCredentials(const char
*pkipath,
> VIR_FREE(userdir);
>
> return 0;
> -
> - error:
> - VIR_FREE(*cacert);
> - VIR_FREE(*cacrl);
> - VIR_FREE(*key);
> - VIR_FREE(*cert);
> - VIR_FREE(user_pki_path);
> - VIR_FREE(userdir);
> - return -1;
This doesn't look right. Looks like some leftover from rebase where
you wanted to use g_autofree.
No, actually not. The only usage of `goto error` was when checking the
result of virGetUserDirectory().
By removing the check, we have also to remove the label.
Best Regards,
--
Fabiano Fidêncio
10:25 a.m.
New subject: [libvirt] [PATCH 05/42] rpc: Don't check the output of virGetUserDirectory()
On Thu, Dec 19, 2019 at 05:21:21PM +0100, Fabiano Fidêncio wrote:
On Thu, Dec 19, 2019 at 5:07 PM Pavel Hrdina
<phrdina(a)redhat.com> wrote:
>
> On Thu, Dec 19, 2019 at 11:04:10AM +0100, Fabiano Fidêncio wrote:
> > virGetUserDirectory() *never* *ever* returns NULL, making the checks for
> > it completely unnecessary.
> >
> > Signed-off-by: Fabiano Fidêncio <fidencio(a)redhat.com>
> > ---
> > src/rpc/virnetclient.c | 12 ++++--------
> > src/rpc/virnettlscontext.c | 12 ------------
> > 2 files changed, 4 insertions(+), 20 deletions(-)
>
> [...]
>
> > diff --git a/src/rpc/virnettlscontext.c b/src/rpc/virnettlscontext.c
> > index ec9dd35c46..08944f6771 100644
> > --- a/src/rpc/virnettlscontext.c
> > +++ b/src/rpc/virnettlscontext.c
> > @@ -805,9 +805,6 @@ static int virNetTLSContextLocateCredentials(const char
*pkipath,
> > */
> > userdir = virGetUserDirectory();
> >
> > - if (!userdir)
> > - goto error;
> > -
> > user_pki_path = g_strdup_printf("%s/.pki/libvirt",
userdir);
> >
> > VIR_DEBUG("Trying to find TLS user credentials in %s",
user_pki_path);
> > @@ -864,15 +861,6 @@ static int virNetTLSContextLocateCredentials(const char
*pkipath,
> > VIR_FREE(userdir);
> >
> > return 0;
> > -
> > - error:
> > - VIR_FREE(*cacert);
> > - VIR_FREE(*cacrl);
> > - VIR_FREE(*key);
> > - VIR_FREE(*cert);
> > - VIR_FREE(user_pki_path);
> > - VIR_FREE(userdir);
> > - return -1;
>
> This doesn't look right. Looks like some leftover from rebase where
> you wanted to use g_autofree.
No, actually not. The only usage of `goto error` was when checking the
result of virGetUserDirectory().
By removing the check, we have also to remove the label.
Label yes, the VIR_FREE no as it would leak the memory.
Pavel
10:42 a.m.
New subject: [libvirt] [PATCH 05/42] rpc: Don't check the output of virGetUserDirectory()
On Thu, Dec 19, 2019 at 05:25:59PM +0100, Pavel Hrdina wrote:
On Thu, Dec 19, 2019 at 05:21:21PM +0100, Fabiano Fidêncio wrote:
> On Thu, Dec 19, 2019 at 5:07 PM Pavel Hrdina <phrdina(a)redhat.com> wrote:
> >
> > On Thu, Dec 19, 2019 at 11:04:10AM +0100, Fabiano Fidêncio wrote:
> > > virGetUserDirectory() *never* *ever* returns NULL, making the checks for
> > > it completely unnecessary.
> > >
> > > Signed-off-by: Fabiano Fidêncio <fidencio(a)redhat.com>
> > > ---
> > > src/rpc/virnetclient.c | 12 ++++--------
> > > src/rpc/virnettlscontext.c | 12 ------------
> > > 2 files changed, 4 insertions(+), 20 deletions(-)
> >
> > [...]
> >
> > > diff --git a/src/rpc/virnettlscontext.c b/src/rpc/virnettlscontext.c
> > > index ec9dd35c46..08944f6771 100644
> > > --- a/src/rpc/virnettlscontext.c
> > > +++ b/src/rpc/virnettlscontext.c
> > > @@ -805,9 +805,6 @@ static int virNetTLSContextLocateCredentials(const char
*pkipath,
> > > */
> > > userdir = virGetUserDirectory();
> > >
> > > - if (!userdir)
> > > - goto error;
> > > -
> > > user_pki_path = g_strdup_printf("%s/.pki/libvirt",
userdir);
> > >
> > > VIR_DEBUG("Trying to find TLS user credentials in %s",
user_pki_path);
> > > @@ -864,15 +861,6 @@ static int virNetTLSContextLocateCredentials(const
char *pkipath,
> > > VIR_FREE(userdir);
> > >
> > > return 0;
> > > -
> > > - error:
> > > - VIR_FREE(*cacert);
> > > - VIR_FREE(*cacrl);
> > > - VIR_FREE(*key);
> > > - VIR_FREE(*cert);
> > > - VIR_FREE(user_pki_path);
> > > - VIR_FREE(userdir);
> > > - return -1;
> >
> > This doesn't look right. Looks like some leftover from rebase where
> > you wanted to use g_autofree.
>
> No, actually not. The only usage of `goto error` was when checking the
> result of virGetUserDirectory().
> By removing the check, we have also to remove the label.
Label yes, the VIR_FREE no as it would leak the memory.
There's "return 0;" right before that label.
Jano
Pavel
--
libvir-list mailing list
libvir-list(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
10:44 a.m.
New subject: [libvirt] [PATCH 05/42] rpc: Don't check the output of virGetUserDirectory()
On Thu, Dec 19, 2019 at 05:42:14PM +0100, Ján Tomko wrote:
On Thu, Dec 19, 2019 at 05:25:59PM +0100, Pavel Hrdina wrote:
> On Thu, Dec 19, 2019 at 05:21:21PM +0100, Fabiano Fidêncio wrote:
> > On Thu, Dec 19, 2019 at 5:07 PM Pavel Hrdina <phrdina(a)redhat.com> wrote:
> > >
> > > On Thu, Dec 19, 2019 at 11:04:10AM +0100, Fabiano Fidêncio wrote:
> > > > virGetUserDirectory() *never* *ever* returns NULL, making the checks
for
> > > > it completely unnecessary.
> > > >
> > > > Signed-off-by: Fabiano Fidêncio <fidencio(a)redhat.com>
> > > > ---
> > > > src/rpc/virnetclient.c | 12 ++++--------
> > > > src/rpc/virnettlscontext.c | 12 ------------
> > > > 2 files changed, 4 insertions(+), 20 deletions(-)
> > >
> > > [...]
> > >
> > > > diff --git a/src/rpc/virnettlscontext.c b/src/rpc/virnettlscontext.c
> > > > index ec9dd35c46..08944f6771 100644
> > > > --- a/src/rpc/virnettlscontext.c
> > > > +++ b/src/rpc/virnettlscontext.c
> > > > @@ -805,9 +805,6 @@ static int
virNetTLSContextLocateCredentials(const char *pkipath,
> > > > */
> > > > userdir = virGetUserDirectory();
> > > >
> > > > - if (!userdir)
> > > > - goto error;
> > > > -
> > > > user_pki_path = g_strdup_printf("%s/.pki/libvirt",
userdir);
> > > >
> > > > VIR_DEBUG("Trying to find TLS user credentials in
%s", user_pki_path);
> > > > @@ -864,15 +861,6 @@ static int
virNetTLSContextLocateCredentials(const char *pkipath,
> > > > VIR_FREE(userdir);
> > > >
> > > > return 0;
> > > > -
> > > > - error:
> > > > - VIR_FREE(*cacert);
> > > > - VIR_FREE(*cacrl);
> > > > - VIR_FREE(*key);
> > > > - VIR_FREE(*cert);
> > > > - VIR_FREE(user_pki_path);
> > > > - VIR_FREE(userdir);
> > > > - return -1;
> > >
> > > This doesn't look right. Looks like some leftover from rebase where
> > > you wanted to use g_autofree.
> >
> > No, actually not. The only usage of `goto error` was when checking the
> > result of virGetUserDirectory().
> > By removing the check, we have also to remove the label.
>
> Label yes, the VIR_FREE no as it would leak the memory.
>
There's "return 0;" right before that label.
Oh, I see, two strings are freed and the remaining ones are
transferred to caller. Somehow I missed that.
Pavel
10:40 a.m.
New subject: [libvirt] [PATCH 05/42] rpc: Don't check the output of virGetUserDirectory()
On Thu, Dec 19, 2019 at 11:04:10AM +0100, Fabiano Fidêncio wrote:
virGetUserDirectory() *never* *ever* returns NULL, making the checks
for
it completely unnecessary.
Signed-off-by: Fabiano Fidêncio <fidencio(a)redhat.com>
---
src/rpc/virnetclient.c | 12 ++++--------
src/rpc/virnettlscontext.c | 12 ------------
2 files changed, 4 insertions(+), 20 deletions(-)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
4:04 a.m.
New subject: [libvirt] [PATCH 06/42] qemu: Don't check the output of virGetUserDirectory()
virGetUserDirectory() *never* *ever* returns NULL, making the checks for
it completely unnecessary.
Signed-off-by: Fabiano Fidêncio <fidencio(a)redhat.com>
---
src/qemu/qemu_interop_config.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/src/qemu/qemu_interop_config.c b/src/qemu/qemu_interop_config.c
index cbabde45df..f5f419e630 100644
--- a/src/qemu/qemu_interop_config.c
+++ b/src/qemu/qemu_interop_config.c
@@ -118,9 +118,6 @@ qemuInteropFetchConfigs(const char *name,
if (!xdgConfig) {
g_autofree char *home = virGetUserDirectory();
- if (!home)
- return -1;
-
xdgConfig = g_strdup_printf("%s/.config", home);
}
--
2.24.1
10:43 a.m.
New subject: [libvirt] [PATCH 06/42] qemu: Don't check the output of virGetUserDirectory()
On Thu, Dec 19, 2019 at 11:04:11AM +0100, Fabiano Fidêncio wrote:
virGetUserDirectory() *never* *ever* returns NULL, making the checks
for
it completely unnecessary.
Signed-off-by: Fabiano Fidêncio <fidencio(a)redhat.com>
---
src/qemu/qemu_interop_config.c | 3 ---
1 file changed, 3 deletions(-)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
4:04 a.m.
New subject: [libvirt] [PATCH 07/42] util: Don't check the output of virGetUserConfigDirectory()
virGetUserConfigDirectory() *never* *ever* returns NULL, making the
checks for it completely unnecessary.
Signed-off-by: Fabiano Fidêncio <fidencio(a)redhat.com>
---
src/util/virauth.c | 3 +--
src/util/virconf.c | 2 --
2 files changed, 1 insertion(+), 4 deletions(-)
diff --git a/src/util/virauth.c b/src/util/virauth.c
index 55208c01ef..f75e674586 100644
--- a/src/util/virauth.c
+++ b/src/util/virauth.c
@@ -66,8 +66,7 @@ virAuthGetConfigFilePathURI(virURIPtr uri,
}
}
- if (!(userdir = virGetUserConfigDirectory()))
- return -1;
+ userdir = virGetUserConfigDirectory();
*path = g_strdup_printf("%s/auth.conf", userdir);
diff --git a/src/util/virconf.c b/src/util/virconf.c
index dce84cabb7..cd18ea61e6 100644
--- a/src/util/virconf.c
+++ b/src/util/virconf.c
@@ -1507,8 +1507,6 @@ virConfLoadConfigPath(const char *name)
path = g_strdup_printf("%s/libvirt/%s", SYSCONFDIR, name);
} else {
char *userdir = virGetUserConfigDirectory();
- if (!userdir)
- return NULL;
path = g_strdup_printf("%s/%s", userdir, name);
VIR_FREE(userdir);
--
2.24.1
10:45 a.m.
New subject: [libvirt] [PATCH 07/42] util: Don't check the output of virGetUserConfigDirectory()
On Thu, Dec 19, 2019 at 11:04:12AM +0100, Fabiano Fidêncio wrote:
virGetUserConfigDirectory() *never* *ever* returns NULL, making the
checks for it completely unnecessary.
Signed-off-by: Fabiano Fidêncio <fidencio(a)redhat.com>
---
src/util/virauth.c | 3 +--
src/util/virconf.c | 2 --
2 files changed, 1 insertion(+), 4 deletions(-)
diff --git a/src/util/virconf.c b/src/util/virconf.c
index dce84cabb7..cd18ea61e6 100644
--- a/src/util/virconf.c
+++ b/src/util/virconf.c
@@ -1507,8 +1507,6 @@ virConfLoadConfigPath(const char *name)
path = g_strdup_printf("%s/libvirt/%s", SYSCONFDIR, name);
} else {
char *userdir = virGetUserConfigDirectory();
- if (!userdir)
- return NULL;
Missed g_autofree opportunity
path = g_strdup_printf("%s/%s", userdir, name);
VIR_FREE(userdir);
Whether you squash it in or not:
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
4:04 a.m.
New subject: [libvirt] [PATCH 08/42] storage: Don't check the output of virGetUserConfigDirectory()
virGetUserConfigDirectory() *never* *ever* returns NULL, making the
checks for it completely unnecessary.
Signed-off-by: Fabiano Fidêncio <fidencio(a)redhat.com>
---
src/storage/storage_driver.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index 580a5e6f15..71078dfbd6 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -278,7 +278,7 @@ storageStateInitialize(bool privileged,
} else {
configdir = virGetUserConfigDirectory();
rundir = virGetUserRuntimeDirectory();
- if (!(configdir && rundir))
+ if (!rundir)
goto error;
driver->configDir = g_strdup_printf("%s/storage", configdir);
--
2.24.1
10:50 a.m.
New subject: [libvirt] [PATCH 08/42] storage: Don't check the output of virGetUserConfigDirectory()
On Thu, Dec 19, 2019 at 11:04:13AM +0100, Fabiano Fidêncio wrote:
virGetUserConfigDirectory() *never* *ever* returns NULL, making the
checks for it completely unnecessary.
Signed-off-by: Fabiano Fidêncio <fidencio(a)redhat.com>
---
src/storage/storage_driver.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index 580a5e6f15..71078dfbd6 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -278,7 +278,7 @@ storageStateInitialize(bool privileged,
} else {
configdir = virGetUserConfigDirectory();
rundir = virGetUserRuntimeDirectory();
- if (!(configdir && rundir))
+ if (!rundir)
goto error;
Looking at virGetUserRuntimeDirectory, that one should always return as
well, so you can delete them both (adjusting the commit message)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
11:24 a.m.
New subject: [libvirt] [PATCH 08/42] storage: Don't check the output of virGetUserConfigDirectory()
On Thu, Dec 19, 2019 at 5:51 PM Ján Tomko <jtomko(a)redhat.com> wrote:
On Thu, Dec 19, 2019 at 11:04:13AM +0100, Fabiano Fidêncio wrote:
>virGetUserConfigDirectory() *never* *ever* returns NULL, making the
>checks for it completely unnecessary.
>
>Signed-off-by: Fabiano Fidêncio <fidencio(a)redhat.com>
>---
> src/storage/storage_driver.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
>index 580a5e6f15..71078dfbd6 100644
>--- a/src/storage/storage_driver.c
>+++ b/src/storage/storage_driver.c
>@@ -278,7 +278,7 @@ storageStateInitialize(bool privileged,
> } else {
> configdir = virGetUserConfigDirectory();
> rundir = virGetUserRuntimeDirectory();
>- if (!(configdir && rundir))
>+ if (!rundir)
> goto error;
>
Looking at virGetUserRuntimeDirectory, that one should always return as
well, so you can delete them both (adjusting the commit message)
It's done later in the series, when I change all the
virGetUserRuntimeDirectory() cases.
Best Regards,
--
Fabiano Fidêncio
4:04 a.m.
New subject: [libvirt] [PATCH 09/42] secret: Don't check the output of virGetUserConfigDirectory()
virGetUserConfigDirectory() *never* *ever* returns NULL, making the
checks for it completely unnecessary.
Signed-off-by: Fabiano Fidêncio <fidencio(a)redhat.com>
---
src/secret/secret_driver.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/src/secret/secret_driver.c b/src/secret/secret_driver.c
index 93b4256450..cdc4b7dcf9 100644
--- a/src/secret/secret_driver.c
+++ b/src/secret/secret_driver.c
@@ -475,8 +475,7 @@ secretStateInitialize(bool privileged,
g_autofree char *rundir = NULL;
g_autofree char *cfgdir = NULL;
- if (!(cfgdir = virGetUserConfigDirectory()))
- goto error;
+ cfgdir = virGetUserConfigDirectory();
driver->configDir = g_strdup_printf("%s/secrets/", cfgdir);
if (!(rundir = virGetUserRuntimeDirectory()))
--
2.24.1
10:51 a.m.
New subject: [libvirt] [PATCH 09/42] secret: Don't check the output of virGetUserConfigDirectory()
On Thu, Dec 19, 2019 at 11:04:14AM +0100, Fabiano Fidêncio wrote:
virGetUserConfigDirectory() *never* *ever* returns NULL, making the
checks for it completely unnecessary.
Signed-off-by: Fabiano Fidêncio <fidencio(a)redhat.com>
---
src/secret/secret_driver.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
4:04 a.m.
New subject: [libvirt] [PATCH 10/42] tools: Don't check the output of virGetUserCacheDirectory()
virGetUserCacheDirectory() *never* *ever* returns NULL, making the
checks for it completely unnecessary.
Signed-off-by: Fabiano Fidêncio <fidencio(a)redhat.com>
---
tools/vsh.c | 5 -----
1 file changed, 5 deletions(-)
diff --git a/tools/vsh.c b/tools/vsh.c
index bbb6227130..b982aeb359 100644
--- a/tools/vsh.c
+++ b/tools/vsh.c
@@ -2913,11 +2913,6 @@ vshReadlineInit(vshControl *ctl)
*/
userdir = virGetUserCacheDirectory();
- if (userdir == NULL) {
- vshError(ctl, "%s", _("Could not determine home
directory"));
- goto cleanup;
- }
-
ctl->historydir = g_strdup_printf("%s/%s", userdir, ctl->name);
ctl->historyfile = g_strdup_printf("%s/history", ctl->historydir);
--
2.24.1
1:14 p.m.
New subject: [libvirt] [PATCH 10/42] tools: Don't check the output of virGetUserCacheDirectory()
On Thu, Dec 19, 2019 at 11:04:15AM +0100, Fabiano Fidêncio wrote:
virGetUserCacheDirectory() *never* *ever* returns NULL, making the
checks for it completely unnecessary.
Signed-off-by: Fabiano Fidêncio <fidencio(a)redhat.com>
---
tools/vsh.c | 5 -----
1 file changed, 5 deletions(-)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
4:04 a.m.
New subject: [libvirt] [PATCH 11/42] vbox: Use g_autofree on vboxDomainScreenshot()
This also fixes a cacheDir's leak when g_mkstep_full() fails.
Signed-off-by: Fabiano Fidêncio <fidencio(a)redhat.com>
---
src/vbox/vbox_common.c | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c
index 5957ae52c4..586937fa19 100644
--- a/src/vbox/vbox_common.c
+++ b/src/vbox/vbox_common.c
@@ -7341,8 +7341,8 @@ vboxDomainScreenshot(virDomainPtr dom,
vboxIID iid;
IMachine *machine = NULL;
nsresult rc;
- char *tmp;
- char *cacheDir;
+ g_autofree char *tmp = NULL;
+ g_autofree char *cacheDir = NULL;
int tmp_fd = -1;
unsigned int max_screen;
bool privileged = geteuid() == 0;
@@ -7383,7 +7383,6 @@ vboxDomainScreenshot(virDomainPtr dom,
if ((tmp_fd = g_mkstemp_full(tmp, O_RDWR | O_CLOEXEC, S_IRUSR | S_IWUSR)) == -1) {
virReportSystemError(errno, _("g_mkstemp(\"%s\") failed"),
tmp);
- VIR_FREE(tmp);
VBOX_RELEASE(machine);
return NULL;
}
@@ -7454,8 +7453,6 @@ vboxDomainScreenshot(virDomainPtr dom,
VIR_FORCE_CLOSE(tmp_fd);
unlink(tmp);
- VIR_FREE(tmp);
- VIR_FREE(cacheDir);
VBOX_RELEASE(machine);
vboxIIDUnalloc(&iid);
return ret;
--
2.24.1
1:16 p.m.
New subject: [libvirt] [PATCH 11/42] vbox: Use g_autofree on vboxDomainScreenshot()
On Thu, Dec 19, 2019 at 11:04:16AM +0100, Fabiano Fidêncio wrote:
This also fixes a cacheDir's leak when g_mkstep_full() fails.
Signed-off-by: Fabiano Fidêncio <fidencio(a)redhat.com>
---
src/vbox/vbox_common.c | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
4:04 a.m.
New subject: [libvirt] [PATCH 12/42] vbox: Don't check the output of virGetUserCacheDirectory()
virGetUserCacheDirectory() *never* *ever* returns NULL, making the
checks for it completely unnecessary.
Signed-off-by: Fabiano Fidêncio <fidencio(a)redhat.com>
---
src/vbox/vbox_common.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c
index 586937fa19..4493fe8582 100644
--- a/src/vbox/vbox_common.c
+++ b/src/vbox/vbox_common.c
@@ -7374,9 +7374,8 @@ vboxDomainScreenshot(virDomainPtr dom,
if (privileged) {
cacheDir = g_strdup_printf("%s/cache/libvirt", LOCALSTATEDIR);
- } else if (!(cacheDir = virGetUserCacheDirectory())) {
- VBOX_RELEASE(machine);
- return NULL;
+ } else {
+ cacheDir = virGetUserCacheDirectory();
}
tmp = g_strdup_printf("%s/vbox.screendump.XXXXXX", cacheDir);
--
2.24.1
1:17 p.m.
New subject: [libvirt] [PATCH 12/42] vbox: Don't check the output of virGetUserCacheDirectory()
On Thu, Dec 19, 2019 at 11:04:17AM +0100, Fabiano Fidêncio wrote:
virGetUserCacheDirectory() *never* *ever* returns NULL, making the
checks for it completely unnecessary.
Signed-off-by: Fabiano Fidêncio <fidencio(a)redhat.com>
---
src/vbox/vbox_common.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
4:04 a.m.
New subject: [libvirt] [PATCH 13/42] util: Use g_autofree on virLogSetDefaultOutputToFile()
Signed-off-by: Fabiano Fidêncio <fidencio(a)redhat.com>
---
src/util/virlog.c | 12 ++++--------
1 file changed, 4 insertions(+), 8 deletions(-)
diff --git a/src/util/virlog.c b/src/util/virlog.c
index d45e2dd316..10639d7328 100644
--- a/src/util/virlog.c
+++ b/src/util/virlog.c
@@ -173,8 +173,7 @@ virLogSetDefaultOutputToJournald(void)
static int
virLogSetDefaultOutputToFile(const char *binary, bool privileged)
{
- int ret = -1;
- char *logdir = NULL;
+ g_autofree char *logdir = NULL;
mode_t old_umask;
if (privileged) {
@@ -182,12 +181,12 @@ virLogSetDefaultOutputToFile(const char *binary, bool privileged)
virLogDefaultPriority, LOCALSTATEDIR,
binary);
} else {
if (!(logdir = virGetUserCacheDirectory()))
- goto cleanup;
+ return -1;
old_umask = umask(077);
if (virFileMakePath(logdir) < 0) {
umask(old_umask);
- goto cleanup;
+ return -1;
}
umask(old_umask);
@@ -195,10 +194,7 @@ virLogSetDefaultOutputToFile(const char *binary, bool privileged)
virLogDefaultPriority, logdir, binary);
}
- ret = 0;
- cleanup:
- VIR_FREE(logdir);
- return ret;
+ return 0;
}
--
2.24.1
1:19 p.m.
New subject: [libvirt] [PATCH 13/42] util: Use g_autofree on virLogSetDefaultOutputToFile()
s/on/in/
On Thu, Dec 19, 2019 at 11:04:18AM +0100, Fabiano Fidêncio wrote:
Signed-off-by: Fabiano Fidêncio <fidencio(a)redhat.com>
---
src/util/virlog.c | 12 ++++--------
1 file changed, 4 insertions(+), 8 deletions(-)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
4:04 a.m.
New subject: [libvirt] [PATCH 14/42] util: Don't check the output of virGetUserCacheDirectory()
virGetUserCacheDirectory() *never* *ever* returns NULL, making the
checks for it completely unnecessary.
Signed-off-by: Fabiano Fidêncio <fidencio(a)redhat.com>
---
src/util/virlog.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/src/util/virlog.c b/src/util/virlog.c
index 10639d7328..6bae56e2e3 100644
--- a/src/util/virlog.c
+++ b/src/util/virlog.c
@@ -180,8 +180,7 @@ virLogSetDefaultOutputToFile(const char *binary, bool privileged)
virLogDefaultOutput = g_strdup_printf("%d:file:%s/log/libvirt/%s.log",
virLogDefaultPriority, LOCALSTATEDIR,
binary);
} else {
- if (!(logdir = virGetUserCacheDirectory()))
- return -1;
+ logdir = virGetUserCacheDirectory();
old_umask = umask(077);
if (virFileMakePath(logdir) < 0) {
--
2.24.1
1:19 p.m.
New subject: [libvirt] [PATCH 14/42] util: Don't check the output of virGetUserCacheDirectory()
On Thu, Dec 19, 2019 at 11:04:19AM +0100, Fabiano Fidêncio wrote:
virGetUserCacheDirectory() *never* *ever* returns NULL, making the
checks for it completely unnecessary.
Signed-off-by: Fabiano Fidêncio <fidencio(a)redhat.com>
---
src/util/virlog.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
4:04 a.m.
New subject: [libvirt] [PATCH 15/42] qemu: Don't check the output of virGetUserCacheDirectory()
virGetUserCacheDirectory() *never* *ever* returns NULL, making the
checks for it completely unnecessary.
Signed-off-by: Fabiano Fidêncio <fidencio(a)redhat.com>
---
src/qemu/qemu_conf.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 389ac55bee..634c65095e 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -165,8 +165,6 @@ virQEMUDriverConfigPtr virQEMUDriverConfigNew(bool privileged)
g_autofree char *cachedir = NULL;
cachedir = virGetUserCacheDirectory();
- if (!cachedir)
- return NULL;
cfg->logDir = g_strdup_printf("%s/qemu/log", cachedir);
cfg->swtpmLogDir = g_strdup_printf("%s/qemu/log", cachedir);
--
2.24.1
1:19 p.m.
New subject: [libvirt] [PATCH 15/42] qemu: Don't check the output of virGetUserCacheDirectory()
On Thu, Dec 19, 2019 at 11:04:20AM +0100, Fabiano Fidêncio wrote:
virGetUserCacheDirectory() *never* *ever* returns NULL, making the
checks for it completely unnecessary.
Signed-off-by: Fabiano Fidêncio <fidencio(a)redhat.com>
---
src/qemu/qemu_conf.c | 2 --
1 file changed, 2 deletions(-)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
4:04 a.m.
New subject: [libvirt] [PATCH 16/42] rpc: Don't check the output of virGetUserConfigDirectory()
virGetUserConfigDirectory() *never* *ever* returns NULL, making the
checks for it completely unnecessary.
Signed-off-by: Fabiano Fidêncio <fidencio(a)redhat.com>
---
src/rpc/virnetclient.c | 11 ++++-------
1 file changed, 4 insertions(+), 7 deletions(-)
diff --git a/src/rpc/virnetclient.c b/src/rpc/virnetclient.c
index 40e5fa35e2..eba8b865d1 100644
--- a/src/rpc/virnetclient.c
+++ b/src/rpc/virnetclient.c
@@ -455,11 +455,9 @@ virNetClientPtr virNetClientNewLibSSH2(const char *host,
knownhosts = g_strdup(knownHostsPath);
} else {
confdir = virGetUserConfigDirectory();
- if (confdir) {
- virBufferAsprintf(&buf, "%s/known_hosts", confdir);
- if (!(knownhosts = virBufferContentAndReset(&buf)))
- goto no_memory;
- }
+ virBufferAsprintf(&buf, "%s/known_hosts", confdir);
+ if (!(knownhosts = virBufferContentAndReset(&buf)))
+ goto no_memory;
}
if (privkeyPath) {
@@ -556,8 +554,7 @@ virNetClientPtr virNetClientNewLibssh(const char *host,
knownhosts = g_strdup(knownHostsPath);
} else {
confdir = virGetUserConfigDirectory();
- if (confdir)
- knownhosts = g_strdup_printf("%s/known_hosts", confdir);
+ knownhosts = g_strdup_printf("%s/known_hosts", confdir);
}
if (privkeyPath) {
--
2.24.1
1:20 p.m.
New subject: [libvirt] [PATCH 16/42] rpc: Don't check the output of virGetUserConfigDirectory()
On Thu, Dec 19, 2019 at 11:04:21AM +0100, Fabiano Fidêncio wrote:
virGetUserConfigDirectory() *never* *ever* returns NULL, making the
checks for it completely unnecessary.
Signed-off-by: Fabiano Fidêncio <fidencio(a)redhat.com>
---
src/rpc/virnetclient.c | 11 ++++-------
1 file changed, 4 insertions(+), 7 deletions(-)
diff --git a/src/rpc/virnetclient.c b/src/rpc/virnetclient.c
index 40e5fa35e2..eba8b865d1 100644
--- a/src/rpc/virnetclient.c
+++ b/src/rpc/virnetclient.c
@@ -455,11 +455,9 @@ virNetClientPtr virNetClientNewLibSSH2(const char *host,
knownhosts = g_strdup(knownHostsPath);
} else {
confdir = virGetUserConfigDirectory();
- if (confdir) {
- virBufferAsprintf(&buf, "%s/known_hosts", confdir);
- if (!(knownhosts = virBufferContentAndReset(&buf)))
- goto no_memory;
- }
+ virBufferAsprintf(&buf, "%s/known_hosts", confdir);
+ if (!(knownhosts = virBufferContentAndReset(&buf)))
+ goto no_memory;
no_memory seems suspicious in the times of abort()
}
if (privkeyPath) {
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
4:45 p.m.
New subject: [libvirt] [PATCH 16/42] rpc: Don't check the output of virGetUserConfigDirectory()
On Thu, Dec 19, 2019 at 8:23 PM Ján Tomko <jtomko(a)redhat.com> wrote:
On Thu, Dec 19, 2019 at 11:04:21AM +0100, Fabiano Fidêncio wrote:
>virGetUserConfigDirectory() *never* *ever* returns NULL, making the
>checks for it completely unnecessary.
>
>Signed-off-by: Fabiano Fidêncio <fidencio(a)redhat.com>
>---
> src/rpc/virnetclient.c | 11 ++++-------
> 1 file changed, 4 insertions(+), 7 deletions(-)
>
>diff --git a/src/rpc/virnetclient.c b/src/rpc/virnetclient.c
>index 40e5fa35e2..eba8b865d1 100644
>--- a/src/rpc/virnetclient.c
>+++ b/src/rpc/virnetclient.c
>@@ -455,11 +455,9 @@ virNetClientPtr virNetClientNewLibSSH2(const char *host,
> knownhosts = g_strdup(knownHostsPath);
> } else {
> confdir = virGetUserConfigDirectory();
>- if (confdir) {
>- virBufferAsprintf(&buf, "%s/known_hosts", confdir);
>- if (!(knownhosts = virBufferContentAndReset(&buf)))
>- goto no_memory;
>- }
>+ virBufferAsprintf(&buf, "%s/known_hosts", confdir);
>+ if (!(knownhosts = virBufferContentAndReset(&buf)))
>+ goto no_memory;
no_memory seems suspicious in the times of abort()
Indeed. But that's material for another series.
Best Regards,
--
Fabiano Fidêncio
4:04 a.m.
New subject: [libvirt] [PATCH 17/42] remote: Don't check the output of virGetUserConfigDirectory()
virGetUserConfigDirectory() *never* *ever* returns NULL, making the
checks for it completely unnecessary.
Signed-off-by: Fabiano Fidêncio <fidencio(a)redhat.com>
---
src/remote/remote_daemon_config.c | 6 +-----
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/src/remote/remote_daemon_config.c b/src/remote/remote_daemon_config.c
index ae6b491686..84e331474b 100644
--- a/src/remote/remote_daemon_config.c
+++ b/src/remote/remote_daemon_config.c
@@ -81,17 +81,13 @@ daemonConfigFilePath(bool privileged, char **configfile)
} else {
char *configdir = NULL;
- if (!(configdir = virGetUserConfigDirectory()))
- goto error;
+ configdir = virGetUserConfigDirectory();
*configfile = g_strdup_printf("%s/%s.conf", configdir, DAEMON_NAME);
VIR_FREE(configdir);
}
return 0;
-
- error:
- return -1;
}
struct daemonConfig*
--
2.24.1
1:21 p.m.
New subject: [libvirt] [PATCH 17/42] remote: Don't check the output of virGetUserConfigDirectory()
On Thu, Dec 19, 2019 at 11:04:22AM +0100, Fabiano Fidêncio wrote:
virGetUserConfigDirectory() *never* *ever* returns NULL, making the
checks for it completely unnecessary.
Signed-off-by: Fabiano Fidêncio <fidencio(a)redhat.com>
---
src/remote/remote_daemon_config.c | 6 +-----
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/src/remote/remote_daemon_config.c b/src/remote/remote_daemon_config.c
index ae6b491686..84e331474b 100644
--- a/src/remote/remote_daemon_config.c
+++ b/src/remote/remote_daemon_config.c
@@ -81,17 +81,13 @@ daemonConfigFilePath(bool privileged, char **configfile)
} else {
char *configdir = NULL;
- if (!(configdir = virGetUserConfigDirectory()))
- goto error;
+ configdir = virGetUserConfigDirectory();
*configfile = g_strdup_printf("%s/%s.conf", configdir, DAEMON_NAME);
VIR_FREE(configdir);
Another g_autofree opportunity
}
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
4:04 a.m.
New subject: [libvirt] [PATCH 18/42] qemu: Don't check the output of virGetUserConfigDirectory()
virGetUserConfigDirectory() *never* *ever* returns NULL, making the
checks for it completely unnecessary.
Signed-off-by: Fabiano Fidêncio <fidencio(a)redhat.com>
---
src/qemu/qemu_conf.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 634c65095e..aa96d50e41 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -177,8 +177,7 @@ virQEMUDriverConfigPtr virQEMUDriverConfigNew(bool privileged)
cfg->swtpmStateDir = g_strdup_printf("%s/swtpm", cfg->stateDir);
- if (!(cfg->configBaseDir = virGetUserConfigDirectory()))
- return NULL;
+ cfg->configBaseDir = virGetUserConfigDirectory();
cfg->libDir = g_strdup_printf("%s/qemu/lib",
cfg->configBaseDir);
cfg->saveDir = g_strdup_printf("%s/qemu/save",
cfg->configBaseDir);
--
2.24.1
1:22 p.m.
New subject: [libvirt] [PATCH 18/42] qemu: Don't check the output of virGetUserConfigDirectory()
On Thu, Dec 19, 2019 at 11:04:23AM +0100, Fabiano Fidêncio wrote:
virGetUserConfigDirectory() *never* *ever* returns NULL, making the
checks for it completely unnecessary.
Signed-off-by: Fabiano Fidêncio <fidencio(a)redhat.com>
---
src/qemu/qemu_conf.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
4:04 a.m.
New subject: [libvirt] [PATCH 19/42] network: Don't check the output of virGetUserConfigDirectory()
virGetUserConfigDirectory() *never* *ever* returns NULL, making the
checks for it completely unnecessary.
Signed-off-by: Fabiano Fidêncio <fidencio(a)redhat.com>
---
src/network/bridge_driver.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index e360645969..98aa530715 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -741,7 +741,7 @@ networkStateInitialize(bool privileged,
} else {
configdir = virGetUserConfigDirectory();
rundir = virGetUserRuntimeDirectory();
- if (!(configdir && rundir))
+ if (!rundir)
goto error;
network_driver->networkConfigDir =
g_strdup_printf("%s/qemu/networks", configdir);
--
2.24.1
1:22 p.m.
New subject: [libvirt] [PATCH 19/42] network: Don't check the output of virGetUserConfigDirectory()
On Thu, Dec 19, 2019 at 11:04:24AM +0100, Fabiano Fidêncio wrote:
virGetUserConfigDirectory() *never* *ever* returns NULL, making the
checks for it completely unnecessary.
Signed-off-by: Fabiano Fidêncio <fidencio(a)redhat.com>
---
src/network/bridge_driver.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
4:04 a.m.
New subject: [libvirt] [PATCH 20/42] logging: Use g_autofree on virLogDaemonConfigFilePath()
Signed-off-by: Fabiano Fidêncio <fidencio(a)redhat.com>
---
src/logging/log_daemon_config.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/src/logging/log_daemon_config.c b/src/logging/log_daemon_config.c
index 0cf9729e7f..ab42921140 100644
--- a/src/logging/log_daemon_config.c
+++ b/src/logging/log_daemon_config.c
@@ -42,13 +42,12 @@ virLogDaemonConfigFilePath(bool privileged, char **configfile)
if (privileged) {
*configfile = g_strdup(SYSCONFDIR "/libvirt/virtlogd.conf");
} else {
- char *configdir = NULL;
+ g_autofree char *configdir = NULL;
if (!(configdir = virGetUserConfigDirectory()))
goto error;
*configfile = g_strdup_printf("%s/virtlogd.conf", configdir);
- VIR_FREE(configdir);
}
return 0;
--
2.24.1
1:23 p.m.
New subject: [libvirt] [PATCH 20/42] logging: Use g_autofree on virLogDaemonConfigFilePath()
s/on/in/
On Thu, Dec 19, 2019 at 11:04:25AM +0100, Fabiano Fidêncio wrote:
Signed-off-by: Fabiano Fidêncio <fidencio(a)redhat.com>
---
src/logging/log_daemon_config.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
4:04 a.m.
New subject: [libvirt] [PATCH 21/42] logging: Don't check the output of virGetUserConfigDirectory()
virGetUserConfigDirectory() *never* *ever* returns NULL, making the
checks for it completely unnecessary.
Signed-off-by: Fabiano Fidêncio <fidencio(a)redhat.com>
---
src/logging/log_daemon_config.c | 6 +-----
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/src/logging/log_daemon_config.c b/src/logging/log_daemon_config.c
index ab42921140..97f2de90a6 100644
--- a/src/logging/log_daemon_config.c
+++ b/src/logging/log_daemon_config.c
@@ -44,16 +44,12 @@ virLogDaemonConfigFilePath(bool privileged, char **configfile)
} else {
g_autofree char *configdir = NULL;
- if (!(configdir = virGetUserConfigDirectory()))
- goto error;
+ configdir = virGetUserConfigDirectory();
*configfile = g_strdup_printf("%s/virtlogd.conf", configdir);
}
return 0;
-
- error:
- return -1;
}
--
2.24.1
1:23 p.m.
New subject: [libvirt] [PATCH 21/42] logging: Don't check the output of virGetUserConfigDirectory()
On Thu, Dec 19, 2019 at 11:04:26AM +0100, Fabiano Fidêncio wrote:
virGetUserConfigDirectory() *never* *ever* returns NULL, making the
checks for it completely unnecessary.
Signed-off-by: Fabiano Fidêncio <fidencio(a)redhat.com>
---
src/logging/log_daemon_config.c | 6 +-----
1 file changed, 1 insertion(+), 5 deletions(-)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
4:04 a.m.
New subject: [libvirt] [PATCH 22/42] locking: Use g_autofree on virLockDaemonConfigFilePath()
Signed-off-by: Fabiano Fidêncio <fidencio(a)redhat.com>
---
src/locking/lock_daemon_config.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/src/locking/lock_daemon_config.c b/src/locking/lock_daemon_config.c
index d7e13013d7..62df30d9f4 100644
--- a/src/locking/lock_daemon_config.c
+++ b/src/locking/lock_daemon_config.c
@@ -41,13 +41,12 @@ virLockDaemonConfigFilePath(bool privileged, char **configfile)
if (privileged) {
*configfile = g_strdup(SYSCONFDIR "/libvirt/virtlockd.conf");
} else {
- char *configdir = NULL;
+ g_autofree char *configdir = NULL;
if (!(configdir = virGetUserConfigDirectory()))
goto error;
*configfile = g_strdup_printf("%s/virtlockd.conf", configdir);
- VIR_FREE(configdir);
}
return 0;
--
2.24.1
1:23 p.m.
New subject: [libvirt] [PATCH 22/42] locking: Use g_autofree on virLockDaemonConfigFilePath()
On Thu, Dec 19, 2019 at 11:04:27AM +0100, Fabiano Fidêncio wrote:
Signed-off-by: Fabiano Fidêncio <fidencio(a)redhat.com>
---
src/locking/lock_daemon_config.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
4:04 a.m.
New subject: [libvirt] [PATCH 23/42] locking: Don't check the output of virGetUserConfigDirectory()
virGetUserConfigDirectory() *never* *ever* returns NULL, making the
checks for it completely unnecessary.
Signed-off-by: Fabiano Fidêncio <fidencio(a)redhat.com>
---
src/locking/lock_daemon_config.c | 6 +-----
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/src/locking/lock_daemon_config.c b/src/locking/lock_daemon_config.c
index 62df30d9f4..c67b0a75e1 100644
--- a/src/locking/lock_daemon_config.c
+++ b/src/locking/lock_daemon_config.c
@@ -43,16 +43,12 @@ virLockDaemonConfigFilePath(bool privileged, char **configfile)
} else {
g_autofree char *configdir = NULL;
- if (!(configdir = virGetUserConfigDirectory()))
- goto error;
+ configdir = virGetUserConfigDirectory();
*configfile = g_strdup_printf("%s/virtlockd.conf", configdir);
}
return 0;
-
- error:
- return -1;
}
--
2.24.1
1:24 p.m.
New subject: [libvirt] [PATCH 23/42] locking: Don't check the output of virGetUserConfigDirectory()
On Thu, Dec 19, 2019 at 11:04:28AM +0100, Fabiano Fidêncio wrote:
virGetUserConfigDirectory() *never* *ever* returns NULL, making the
checks for it completely unnecessary.
Signed-off-by: Fabiano Fidêncio <fidencio(a)redhat.com>
---
src/locking/lock_daemon_config.c | 6 +-----
1 file changed, 1 insertion(+), 5 deletions(-)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
4:04 a.m.
New subject: [libvirt] [PATCH 24/42] util: Don't check the output of virGetUserRuntimeDirectory()
virGetUserRuntimeDirectory() *never* *ever* returns NULL, making the
checks for it completely unnecessary.
Signed-off-by: Fabiano Fidêncio <fidencio(a)redhat.com>
---
src/util/virhostdev.c | 3 +--
src/util/virpidfile.c | 3 +--
2 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/src/util/virhostdev.c b/src/util/virhostdev.c
index 78e409732a..9b4ea30216 100644
--- a/src/util/virhostdev.c
+++ b/src/util/virhostdev.c
@@ -182,8 +182,7 @@ virHostdevManagerNew(void)
g_autofree char *rundir = NULL;
mode_t old_umask;
- if (!(rundir = virGetUserRuntimeDirectory()))
- return NULL;
+ rundir = virGetUserRuntimeDirectory();
hostdevMgr->stateDir = g_strdup_printf("%s/hostdevmgr", rundir);
diff --git a/src/util/virpidfile.c b/src/util/virpidfile.c
index 249515aff2..b08e0d8d52 100644
--- a/src/util/virpidfile.c
+++ b/src/util/virpidfile.c
@@ -488,8 +488,7 @@ virPidFileConstructPath(bool privileged,
}
*pidfile = g_strdup_printf("%s/%s.pid", runstatedir, progname);
} else {
- if (!(rundir = virGetUserRuntimeDirectory()))
- return -1;
+ rundir = virGetUserRuntimeDirectory();
if (virFileMakePathWithMode(rundir, 0700) < 0) {
virReportSystemError(errno,
--
2.24.1
1:24 p.m.
New subject: [libvirt] [PATCH 24/42] util: Don't check the output of virGetUserRuntimeDirectory()
On Thu, Dec 19, 2019 at 11:04:29AM +0100, Fabiano Fidêncio wrote:
virGetUserRuntimeDirectory() *never* *ever* returns NULL, making the
checks for it completely unnecessary.
Signed-off-by: Fabiano Fidêncio <fidencio(a)redhat.com>
---
src/util/virhostdev.c | 3 +--
src/util/virpidfile.c | 3 +--
2 files changed, 2 insertions(+), 4 deletions(-)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
4:04 a.m.
New subject: [libvirt] [PATCH 25/42] storage: Don't check the output of virGetUserRuntimeDirectory()
virGetUserRuntimeDirectory() *never* *ever* returns NULL, making the
checks for it completely unnecessary.
Signed-off-by: Fabiano Fidêncio <fidencio(a)redhat.com>
---
src/storage/storage_driver.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index 71078dfbd6..a33328db37 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -278,8 +278,6 @@ storageStateInitialize(bool privileged,
} else {
configdir = virGetUserConfigDirectory();
rundir = virGetUserRuntimeDirectory();
- if (!rundir)
- goto error;
driver->configDir = g_strdup_printf("%s/storage", configdir);
driver->autostartDir = g_strdup_printf("%s/storage/autostart",
configdir);
--
2.24.1
1:25 p.m.
New subject: [libvirt] [PATCH 25/42] storage: Don't check the output of virGetUserRuntimeDirectory()
On Thu, Dec 19, 2019 at 11:04:30AM +0100, Fabiano Fidêncio wrote:
virGetUserRuntimeDirectory() *never* *ever* returns NULL, making the
checks for it completely unnecessary.
Signed-off-by: Fabiano Fidêncio <fidencio(a)redhat.com>
---
src/storage/storage_driver.c | 2 --
1 file changed, 2 deletions(-)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
4:04 a.m.
New subject: [libvirt] [PATCH 26/42] secret: Don't check the output of virGetUserRuntimeDirectory()
virGetUserRuntimeDirectory() *never* *ever* returns NULL, making the
checks for it completely unnecessary.
Signed-off-by: Fabiano Fidêncio <fidencio(a)redhat.com>
---
src/secret/secret_driver.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/src/secret/secret_driver.c b/src/secret/secret_driver.c
index cdc4b7dcf9..096672f114 100644
--- a/src/secret/secret_driver.c
+++ b/src/secret/secret_driver.c
@@ -478,8 +478,7 @@ secretStateInitialize(bool privileged,
cfgdir = virGetUserConfigDirectory();
driver->configDir = g_strdup_printf("%s/secrets/", cfgdir);
- if (!(rundir = virGetUserRuntimeDirectory()))
- goto error;
+ rundir = virGetUserRuntimeDirectory();
driver->stateDir = g_strdup_printf("%s/secrets/run", rundir);
}
--
2.24.1
1:25 p.m.
New subject: [libvirt] [PATCH 26/42] secret: Don't check the output of virGetUserRuntimeDirectory()
On Thu, Dec 19, 2019 at 11:04:31AM +0100, Fabiano Fidêncio wrote:
virGetUserRuntimeDirectory() *never* *ever* returns NULL, making the
checks for it completely unnecessary.
Signed-off-by: Fabiano Fidêncio <fidencio(a)redhat.com>
---
src/secret/secret_driver.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
4:04 a.m.
New subject: [libvirt] [PATCH 27/42] rpc: Don't check the output of virGetUserRuntimeDirectory()
virGetUserRuntimeDirectory() *never* *ever* returns NULL, making the
checks for it completely unnecessary.
Signed-off-by: Fabiano Fidêncio <fidencio(a)redhat.com>
---
src/rpc/virnetsocket.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c
index b98287e6d7..f072afe857 100644
--- a/src/rpc/virnetsocket.c
+++ b/src/rpc/virnetsocket.c
@@ -684,8 +684,7 @@ int virNetSocketNewConnectUNIX(const char *path,
goto cleanup;
}
- if (!(rundir = virGetUserRuntimeDirectory()))
- goto cleanup;
+ rundir = virGetUserRuntimeDirectory();
if (virFileMakePathWithMode(rundir, 0700) < 0) {
virReportSystemError(errno,
--
2.24.1
1:25 p.m.
New subject: [libvirt] [PATCH 27/42] rpc: Don't check the output of virGetUserRuntimeDirectory()
On Thu, Dec 19, 2019 at 11:04:32AM +0100, Fabiano Fidêncio wrote:
virGetUserRuntimeDirectory() *never* *ever* returns NULL, making the
checks for it completely unnecessary.
Signed-off-by: Fabiano Fidêncio <fidencio(a)redhat.com>
---
src/rpc/virnetsocket.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
4:04 a.m.
New subject: [libvirt] [PATCH 28/42] remote: Don't check the output of virGetUserRuntimeDirectory()
virGetUserRuntimeDirectory() *never* *ever* returns NULL, making the
checks for it completely unnecessary.
Signed-off-by: Fabiano Fidêncio <fidencio(a)redhat.com>
---
src/remote/remote_daemon.c | 8 +-------
src/remote/remote_driver.c | 3 +--
2 files changed, 2 insertions(+), 9 deletions(-)
diff --git a/src/remote/remote_daemon.c b/src/remote/remote_daemon.c
index b400b1dd10..5893492875 100644
--- a/src/remote/remote_daemon.c
+++ b/src/remote/remote_daemon.c
@@ -245,8 +245,7 @@ daemonUnixSocketPaths(struct daemonConfig *config,
} else {
mode_t old_umask;
- if (!(rundir = virGetUserRuntimeDirectory()))
- goto cleanup;
+ rundir = virGetUserRuntimeDirectory();
old_umask = umask(077);
if (virFileMakePath(rundir) < 0) {
@@ -1208,11 +1207,6 @@ int main(int argc, char **argv) {
run_dir = g_strdup(RUNSTATEDIR "/libvirt");
} else {
run_dir = virGetUserRuntimeDirectory();
-
- if (!run_dir) {
- VIR_ERROR(_("Can't determine user directory"));
- goto cleanup;
- }
}
if (privileged)
old_umask = umask(022);
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index e684fcba09..c11f73ab4d 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -779,8 +779,7 @@ remoteGetUNIXSocketHelper(remoteDriverTransport transport,
remoteDriverTransportTypeToString(transport));
return NULL;
}
- if (!(userdir = virGetUserRuntimeDirectory()))
- return NULL;
+ userdir = virGetUserRuntimeDirectory();
sockname = g_strdup_printf("%s/%s-sock", userdir, sock_prefix);
} else {
--
2.24.1
1:26 p.m.
New subject: [libvirt] [PATCH 28/42] remote: Don't check the output of virGetUserRuntimeDirectory()
On Thu, Dec 19, 2019 at 11:04:33AM +0100, Fabiano Fidêncio wrote:
virGetUserRuntimeDirectory() *never* *ever* returns NULL, making the
checks for it completely unnecessary.
Signed-off-by: Fabiano Fidêncio <fidencio(a)redhat.com>
---
src/remote/remote_daemon.c | 8 +-------
src/remote/remote_driver.c | 3 +--
2 files changed, 2 insertions(+), 9 deletions(-)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
4:04 a.m.
New subject: [libvirt] [PATCH 29/42] qemu: Don't check the output of virGetUserRuntimeDirectory()
virGetUserRuntimeDirectory() *never* *ever* returns NULL, making the
checks for it completely unnecessary.
Signed-off-by: Fabiano Fidêncio <fidencio(a)redhat.com>
---
src/qemu/qemu_conf.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index aa96d50e41..c07a844dfc 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -171,8 +171,6 @@ virQEMUDriverConfigPtr virQEMUDriverConfigNew(bool privileged)
cfg->cacheDir = g_strdup_printf("%s/qemu/cache", cachedir);
rundir = virGetUserRuntimeDirectory();
- if (!rundir)
- return NULL;
cfg->stateDir = g_strdup_printf("%s/qemu/run", rundir);
cfg->swtpmStateDir = g_strdup_printf("%s/swtpm", cfg->stateDir);
--
2.24.1
1:26 p.m.
New subject: [libvirt] [PATCH 29/42] qemu: Don't check the output of virGetUserRuntimeDirectory()
On Thu, Dec 19, 2019 at 11:04:34AM +0100, Fabiano Fidêncio wrote:
virGetUserRuntimeDirectory() *never* *ever* returns NULL, making the
checks for it completely unnecessary.
Signed-off-by: Fabiano Fidêncio <fidencio(a)redhat.com>
---
src/qemu/qemu_conf.c | 2 --
1 file changed, 2 deletions(-)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
4:04 a.m.
New subject: [libvirt] [PATCH 30/42] node_device: Don't check the output of virGetUserRuntimeDirectory()
virGetUserRuntimeDirectory() *never* *ever* returns NULL, making the
checks for it completely unnecessary.
Signed-off-by: Fabiano Fidêncio <fidencio(a)redhat.com>
---
src/node_device/node_device_hal.c | 3 +--
src/node_device/node_device_udev.c | 3 +--
2 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/src/node_device/node_device_hal.c b/src/node_device/node_device_hal.c
index b40f93df46..cf12854fe9 100644
--- a/src/node_device/node_device_hal.c
+++ b/src/node_device/node_device_hal.c
@@ -618,8 +618,7 @@ nodeStateInitialize(bool privileged G_GNUC_UNUSED,
} else {
g_autofree char *rundir = NULL;
- if (!(rundir = virGetUserRuntimeDirectory()))
- goto failure;
+ rundir = virGetUserRuntimeDirectory();
driver->stateDir = g_strdup_printf("%s/nodedev/run", rundir);
}
diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c
index cae00dc9dc..eedcd123a3 100644
--- a/src/node_device/node_device_udev.c
+++ b/src/node_device/node_device_udev.c
@@ -1806,8 +1806,7 @@ nodeStateInitialize(bool privileged,
} else {
g_autofree char *rundir = NULL;
- if (!(rundir = virGetUserRuntimeDirectory()))
- goto cleanup;
+ rundir = virGetUserRuntimeDirectory();
driver->stateDir = g_strdup_printf("%s/nodedev/run", rundir);
}
--
2.24.1
1:27 p.m.
New subject: [libvirt] [PATCH 30/42] node_device: Don't check the output of virGetUserRuntimeDirectory()
On Thu, Dec 19, 2019 at 11:04:35AM +0100, Fabiano Fidêncio wrote:
virGetUserRuntimeDirectory() *never* *ever* returns NULL, making the
checks for it completely unnecessary.
Signed-off-by: Fabiano Fidêncio <fidencio(a)redhat.com>
---
src/node_device/node_device_hal.c | 3 +--
src/node_device/node_device_udev.c | 3 +--
2 files changed, 2 insertions(+), 4 deletions(-)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
4:04 a.m.
New subject: [libvirt] [PATCH 31/42] network: Don't check the output of virGetUserRuntimeDirectory()
virGetUserRuntimeDirectory() *never* *ever* returns NULL, making the
checks for it completely unnecessary.
Signed-off-by: Fabiano Fidêncio <fidencio(a)redhat.com>
---
src/network/bridge_driver.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 98aa530715..c9c45df758 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -741,8 +741,6 @@ networkStateInitialize(bool privileged,
} else {
configdir = virGetUserConfigDirectory();
rundir = virGetUserRuntimeDirectory();
- if (!rundir)
- goto error;
network_driver->networkConfigDir =
g_strdup_printf("%s/qemu/networks", configdir);
network_driver->networkAutostartDir =
g_strdup_printf("%s/qemu/networks/autostart", configdir);
--
2.24.1
1:27 p.m.
New subject: [libvirt] [PATCH 31/42] network: Don't check the output of virGetUserRuntimeDirectory()
On Thu, Dec 19, 2019 at 11:04:36AM +0100, Fabiano Fidêncio wrote:
virGetUserRuntimeDirectory() *never* *ever* returns NULL, making the
checks for it completely unnecessary.
Signed-off-by: Fabiano Fidêncio <fidencio(a)redhat.com>
---
src/network/bridge_driver.c | 2 --
1 file changed, 2 deletions(-)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
4:04 a.m.
New subject: [libvirt] [PATCH 32/42] logging: Use g_autofree on virLogManagerDaemonPath()
Signed-off-by: Fabiano Fidêncio <fidencio(a)redhat.com>
---
src/logging/log_manager.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/src/logging/log_manager.c b/src/logging/log_manager.c
index e191093272..7a4f036240 100644
--- a/src/logging/log_manager.c
+++ b/src/logging/log_manager.c
@@ -47,14 +47,12 @@ virLogManagerDaemonPath(bool privileged)
if (privileged) {
path = g_strdup(RUNSTATEDIR "/libvirt/virtlogd-sock");
} else {
- char *rundir = NULL;
+ g_autofree char *rundir = NULL;
if (!(rundir = virGetUserRuntimeDirectory()))
return NULL;
path = g_strdup_printf("%s/virtlogd-sock", rundir);
-
- VIR_FREE(rundir);
}
return path;
}
--
2.24.1
1:28 p.m.
New subject: [libvirt] [PATCH 32/42] logging: Use g_autofree on virLogManagerDaemonPath()
s/on/in/
On Thu, Dec 19, 2019 at 11:04:37AM +0100, Fabiano Fidêncio wrote:
Signed-off-by: Fabiano Fidêncio <fidencio(a)redhat.com>
---
src/logging/log_manager.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
4:04 a.m.
New subject: [libvirt] [PATCH 33/42] logging: Use g_autofree on virLogDaemonUnixSocketPaths()
Together with the change, let's also simplify the function and get rid
of the goto.
Signed-off-by: Fabiano Fidêncio <fidencio(a)redhat.com>
---
src/logging/log_daemon.c | 12 +++---------
1 file changed, 3 insertions(+), 9 deletions(-)
diff --git a/src/logging/log_daemon.c b/src/logging/log_daemon.c
index 268d3c62b9..d41d9caba9 100644
--- a/src/logging/log_daemon.c
+++ b/src/logging/log_daemon.c
@@ -391,29 +391,23 @@ virLogDaemonUnixSocketPaths(bool privileged,
*sockfile = g_strdup(RUNSTATEDIR "/libvirt/virtlogd-sock");
*adminSockfile = g_strdup(RUNSTATEDIR "/libvirt/virtlogd-admin-sock");
} else {
- char *rundir = NULL;
+ g_autofree char *rundir = NULL;
mode_t old_umask;
if (!(rundir = virGetUserRuntimeDirectory()))
- goto error;
+ return -1;
old_umask = umask(077);
if (virFileMakePath(rundir) < 0) {
umask(old_umask);
- VIR_FREE(rundir);
- goto error;
+ return -1;
}
umask(old_umask);
*sockfile = g_strdup_printf("%s/virtlogd-sock", rundir);
*adminSockfile = g_strdup_printf("%s/virtlogd-admin-sock", rundir);
-
- VIR_FREE(rundir);
}
return 0;
-
- error:
- return -1;
}
--
2.24.1
1:29 p.m.
New subject: [libvirt] [PATCH 33/42] logging: Use g_autofree on virLogDaemonUnixSocketPaths()
On Thu, Dec 19, 2019 at 11:04:38AM +0100, Fabiano Fidêncio wrote:
Together with the change, let's also simplify the function and get
rid
of the goto.
Signed-off-by: Fabiano Fidêncio <fidencio(a)redhat.com>
---
src/logging/log_daemon.c | 12 +++---------
1 file changed, 3 insertions(+), 9 deletions(-)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
4:04 a.m.
New subject: [libvirt] [PATCH 34/42] logging: Use g_autofree on virLogDaemonExecRestartStatePath()
Together with the change, let's also simplify the function and get rid
of the goto.
Signed-off-by: Fabiano Fidêncio <fidencio(a)redhat.com>
---
src/logging/log_daemon.c | 12 +++---------
1 file changed, 3 insertions(+), 9 deletions(-)
diff --git a/src/logging/log_daemon.c b/src/logging/log_daemon.c
index d41d9caba9..3fee2b3b57 100644
--- a/src/logging/log_daemon.c
+++ b/src/logging/log_daemon.c
@@ -612,29 +612,23 @@ virLogDaemonExecRestartStatePath(bool privileged,
if (privileged) {
*state_file = g_strdup(RUNSTATEDIR "/virtlogd-restart-exec.json");
} else {
- char *rundir = NULL;
+ g_autofree char *rundir = NULL;
mode_t old_umask;
if (!(rundir = virGetUserRuntimeDirectory()))
- goto error;
+ return -1;
old_umask = umask(077);
if (virFileMakePath(rundir) < 0) {
umask(old_umask);
- VIR_FREE(rundir);
- goto error;
+ return -1;
}
umask(old_umask);
*state_file = g_strdup_printf("%s/virtlogd-restart-exec.json",
rundir);
-
- VIR_FREE(rundir);
}
return 0;
-
- error:
- return -1;
}
--
2.24.1
1:30 p.m.
New subject: [libvirt] [PATCH 34/42] logging: Use g_autofree on virLogDaemonExecRestartStatePath()
s/on/in/
On Thu, Dec 19, 2019 at 11:04:39AM +0100, Fabiano Fidêncio wrote:
Together with the change, let's also simplify the function and get
rid
of the goto.
Signed-off-by: Fabiano Fidêncio <fidencio(a)redhat.com>
---
src/logging/log_daemon.c | 12 +++---------
1 file changed, 3 insertions(+), 9 deletions(-)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
4:04 a.m.
New subject: [libvirt] [PATCH 35/42] logging: Don't check the output of virGetUserRuntimeDirectory()
virGetUserRuntimeDirectory() *never* *ever* returns NULL, making the
checks for it completely unnecessary.
Signed-off-by: Fabiano Fidêncio <fidencio(a)redhat.com>
---
src/logging/log_daemon.c | 11 +++--------
src/logging/log_manager.c | 3 +--
2 files changed, 4 insertions(+), 10 deletions(-)
diff --git a/src/logging/log_daemon.c b/src/logging/log_daemon.c
index 3fee2b3b57..488f3b459d 100644
--- a/src/logging/log_daemon.c
+++ b/src/logging/log_daemon.c
@@ -394,8 +394,7 @@ virLogDaemonUnixSocketPaths(bool privileged,
g_autofree char *rundir = NULL;
mode_t old_umask;
- if (!(rundir = virGetUserRuntimeDirectory()))
- return -1;
+ rundir = virGetUserRuntimeDirectory();
old_umask = umask(077);
if (virFileMakePath(rundir) < 0) {
@@ -615,8 +614,7 @@ virLogDaemonExecRestartStatePath(bool privileged,
g_autofree char *rundir = NULL;
mode_t old_umask;
- if (!(rundir = virGetUserRuntimeDirectory()))
- return -1;
+ rundir = virGetUserRuntimeDirectory();
old_umask = umask(077);
if (virFileMakePath(rundir) < 0) {
@@ -992,10 +990,7 @@ int main(int argc, char **argv) {
if (privileged) {
run_dir = g_strdup(RUNSTATEDIR "/libvirt");
} else {
- if (!(run_dir = virGetUserRuntimeDirectory())) {
- VIR_ERROR(_("Can't determine user directory"));
- goto cleanup;
- }
+ run_dir = virGetUserRuntimeDirectory();
}
if (privileged)
diff --git a/src/logging/log_manager.c b/src/logging/log_manager.c
index 7a4f036240..6b66218116 100644
--- a/src/logging/log_manager.c
+++ b/src/logging/log_manager.c
@@ -49,8 +49,7 @@ virLogManagerDaemonPath(bool privileged)
} else {
g_autofree char *rundir = NULL;
- if (!(rundir = virGetUserRuntimeDirectory()))
- return NULL;
+ rundir = virGetUserRuntimeDirectory();
path = g_strdup_printf("%s/virtlogd-sock", rundir);
}
--
2.24.1
1:30 p.m.
New subject: [libvirt] [PATCH 35/42] logging: Don't check the output of virGetUserRuntimeDirectory()
On Thu, Dec 19, 2019 at 11:04:40AM +0100, Fabiano Fidêncio wrote:
virGetUserRuntimeDirectory() *never* *ever* returns NULL, making the
checks for it completely unnecessary.
Signed-off-by: Fabiano Fidêncio <fidencio(a)redhat.com>
---
src/logging/log_daemon.c | 11 +++--------
src/logging/log_manager.c | 3 +--
2 files changed, 4 insertions(+), 10 deletions(-)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
4:04 a.m.
New subject: [libvirt] [PATCH 36/42] locking: Use g_autofree on virLockManagerLockDaemonPath()
Signed-off-by: Fabiano Fidêncio <fidencio(a)redhat.com>
---
src/locking/lock_driver_lockd.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/src/locking/lock_driver_lockd.c b/src/locking/lock_driver_lockd.c
index e8f0329b05..8ca77e525d 100644
--- a/src/locking/lock_driver_lockd.c
+++ b/src/locking/lock_driver_lockd.c
@@ -122,14 +122,12 @@ static char *virLockManagerLockDaemonPath(bool privileged)
if (privileged) {
path = g_strdup(RUNSTATEDIR "/libvirt/virtlockd-sock");
} else {
- char *rundir = NULL;
+ g_autofree char *rundir = NULL;
if (!(rundir = virGetUserRuntimeDirectory()))
return NULL;
path = g_strdup_printf("%s/virtlockd-sock", rundir);
-
- VIR_FREE(rundir);
}
return path;
}
--
2.24.1
1:30 p.m.
New subject: [libvirt] [PATCH 36/42] locking: Use g_autofree on virLockManagerLockDaemonPath()
s/on/in/
On Thu, Dec 19, 2019 at 11:04:41AM +0100, Fabiano Fidêncio wrote:
Signed-off-by: Fabiano Fidêncio <fidencio(a)redhat.com>
---
src/locking/lock_driver_lockd.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
4:04 a.m.
New subject: [libvirt] [PATCH 37/42] locking: Use g_autofree on virLockDaemonUnixSocketPaths()
Together with the change, let's also simplify the function and get rid
of the goto.
Signed-off-by: Fabiano Fidêncio <fidencio(a)redhat.com>
---
src/locking/lock_daemon.c | 12 +++---------
1 file changed, 3 insertions(+), 9 deletions(-)
diff --git a/src/locking/lock_daemon.c b/src/locking/lock_daemon.c
index 0d12a97231..9bcd36a869 100644
--- a/src/locking/lock_daemon.c
+++ b/src/locking/lock_daemon.c
@@ -449,29 +449,23 @@ virLockDaemonUnixSocketPaths(bool privileged,
*sockfile = g_strdup(RUNSTATEDIR "/libvirt/virtlockd-sock");
*adminSockfile = g_strdup(RUNSTATEDIR
"/libvirt/virtlockd-admin-sock");
} else {
- char *rundir = NULL;
+ g_autofree char *rundir = NULL;
mode_t old_umask;
if (!(rundir = virGetUserRuntimeDirectory()))
- goto error;
+ return -1;
old_umask = umask(077);
if (virFileMakePath(rundir) < 0) {
- VIR_FREE(rundir);
umask(old_umask);
- goto error;
+ return -1;
}
umask(old_umask);
*sockfile = g_strdup_printf("%s/virtlockd-sock", rundir);
*adminSockfile = g_strdup_printf("%s/virtlockd-admin-sock", rundir);
-
- VIR_FREE(rundir);
}
return 0;
-
- error:
- return -1;
}
--
2.24.1
1:31 p.m.
New subject: [libvirt] [PATCH 37/42] locking: Use g_autofree on virLockDaemonUnixSocketPaths()
On Thu, Dec 19, 2019 at 11:04:42AM +0100, Fabiano Fidêncio wrote:
Together with the change, let's also simplify the function and get
rid
of the goto.
Signed-off-by: Fabiano Fidêncio <fidencio(a)redhat.com>
---
src/locking/lock_daemon.c | 12 +++---------
1 file changed, 3 insertions(+), 9 deletions(-)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
4:04 a.m.
New subject: [libvirt] [PATCH 38/42] locking: Use g_autofree on virLockDaemonExecRestartStatePath()
Together with the change, let's also simplify the function and get rid
of the goto.
Signed-off-by: Fabiano Fidêncio <fidencio(a)redhat.com>
---
src/locking/lock_daemon.c | 12 +++---------
1 file changed, 3 insertions(+), 9 deletions(-)
diff --git a/src/locking/lock_daemon.c b/src/locking/lock_daemon.c
index 9bcd36a869..7c89adf077 100644
--- a/src/locking/lock_daemon.c
+++ b/src/locking/lock_daemon.c
@@ -820,29 +820,23 @@ virLockDaemonExecRestartStatePath(bool privileged,
if (privileged) {
*state_file = g_strdup(RUNSTATEDIR "/virtlockd-restart-exec.json");
} else {
- char *rundir = NULL;
+ g_autofree char *rundir = NULL;
mode_t old_umask;
if (!(rundir = virGetUserRuntimeDirectory()))
- goto error;
+ return -1;
old_umask = umask(077);
if (virFileMakePath(rundir) < 0) {
umask(old_umask);
- VIR_FREE(rundir);
- goto error;
+ return -1;
}
umask(old_umask);
*state_file = g_strdup_printf("%s/virtlockd-restart-exec.json",
rundir);
-
- VIR_FREE(rundir);
}
return 0;
-
- error:
- return -1;
}
--
2.24.1
1:31 p.m.
New subject: [libvirt] [PATCH 38/42] locking: Use g_autofree on virLockDaemonExecRestartStatePath()
On Thu, Dec 19, 2019 at 11:04:43AM +0100, Fabiano Fidêncio wrote:
Together with the change, let's also simplify the function and get
rid
of the goto.
Signed-off-by: Fabiano Fidêncio <fidencio(a)redhat.com>
---
src/locking/lock_daemon.c | 12 +++---------
1 file changed, 3 insertions(+), 9 deletions(-)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
4:04 a.m.
New subject: [libvirt] [PATCH 39/42] locking: Don't check the output of virGetUserRuntimeDirectory()
virGetUserRuntimeDirectory() *never* *ever* returns NULL, making the
checks for it completely unnecessary.
Signed-off-by: Fabiano Fidêncio <fidencio(a)redhat.com>
---
src/locking/lock_daemon.c | 11 +++--------
src/locking/lock_driver_lockd.c | 3 +--
2 files changed, 4 insertions(+), 10 deletions(-)
diff --git a/src/locking/lock_daemon.c b/src/locking/lock_daemon.c
index 7c89adf077..65c38139c4 100644
--- a/src/locking/lock_daemon.c
+++ b/src/locking/lock_daemon.c
@@ -452,8 +452,7 @@ virLockDaemonUnixSocketPaths(bool privileged,
g_autofree char *rundir = NULL;
mode_t old_umask;
- if (!(rundir = virGetUserRuntimeDirectory()))
- return -1;
+ rundir = virGetUserRuntimeDirectory();
old_umask = umask(077);
if (virFileMakePath(rundir) < 0) {
@@ -823,8 +822,7 @@ virLockDaemonExecRestartStatePath(bool privileged,
g_autofree char *rundir = NULL;
mode_t old_umask;
- if (!(rundir = virGetUserRuntimeDirectory()))
- return -1;
+ rundir = virGetUserRuntimeDirectory();
old_umask = umask(077);
if (virFileMakePath(rundir) < 0) {
@@ -1224,10 +1222,7 @@ int main(int argc, char **argv) {
if (privileged) {
run_dir = g_strdup(RUNSTATEDIR "/libvirt");
} else {
- if (!(run_dir = virGetUserRuntimeDirectory())) {
- VIR_ERROR(_("Can't determine user directory"));
- goto cleanup;
- }
+ run_dir = virGetUserRuntimeDirectory();
}
if (privileged)
diff --git a/src/locking/lock_driver_lockd.c b/src/locking/lock_driver_lockd.c
index 8ca77e525d..339e2f6949 100644
--- a/src/locking/lock_driver_lockd.c
+++ b/src/locking/lock_driver_lockd.c
@@ -124,8 +124,7 @@ static char *virLockManagerLockDaemonPath(bool privileged)
} else {
g_autofree char *rundir = NULL;
- if (!(rundir = virGetUserRuntimeDirectory()))
- return NULL;
+ rundir = virGetUserRuntimeDirectory();
path = g_strdup_printf("%s/virtlockd-sock", rundir);
}
--
2.24.1
1:31 p.m.
New subject: [libvirt] [PATCH 39/42] locking: Don't check the output of virGetUserRuntimeDirectory()
On Thu, Dec 19, 2019 at 11:04:44AM +0100, Fabiano Fidêncio wrote:
virGetUserRuntimeDirectory() *never* *ever* returns NULL, making the
checks for it completely unnecessary.
Signed-off-by: Fabiano Fidêncio <fidencio(a)redhat.com>
---
src/locking/lock_daemon.c | 11 +++--------
src/locking/lock_driver_lockd.c | 3 +--
2 files changed, 4 insertions(+), 10 deletions(-)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
4:04 a.m.
New subject: [libvirt] [PATCH 40/42] interface: Don't check the output of virGetUserRuntimeDirectory()
virGetUserRuntimeDirectory() *never* *ever* returns NULL, making the
checks for it completely unnecessary.
Signed-off-by: Fabiano Fidêncio <fidencio(a)redhat.com>
---
src/interface/interface_backend_netcf.c | 3 +--
src/interface/interface_backend_udev.c | 3 +--
2 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/src/interface/interface_backend_netcf.c
b/src/interface/interface_backend_netcf.c
index 4f46717cf3..65cb7eae62 100644
--- a/src/interface/interface_backend_netcf.c
+++ b/src/interface/interface_backend_netcf.c
@@ -105,8 +105,7 @@ netcfStateInitialize(bool privileged,
} else {
g_autofree char *rundir = NULL;
- if (!(rundir = virGetUserRuntimeDirectory()))
- goto error;
+ rundir = virGetUserRuntimeDirectory();
driver->stateDir = g_strdup_printf("%s/interface/run", rundir);
}
diff --git a/src/interface/interface_backend_udev.c
b/src/interface/interface_backend_udev.c
index 26b1045f8a..7cc098eb33 100644
--- a/src/interface/interface_backend_udev.c
+++ b/src/interface/interface_backend_udev.c
@@ -1160,8 +1160,7 @@ udevStateInitialize(bool privileged,
} else {
g_autofree char *rundir = NULL;
- if (!(rundir = virGetUserRuntimeDirectory()))
- goto cleanup;
+ rundir = virGetUserRuntimeDirectory();
driver->stateDir = g_strdup_printf("%s/interface/run", rundir);
}
--
2.24.1
1:32 p.m.
New subject: [libvirt] [PATCH 40/42] interface: Don't check the output of virGetUserRuntimeDirectory()
On Thu, Dec 19, 2019 at 11:04:45AM +0100, Fabiano Fidêncio wrote:
virGetUserRuntimeDirectory() *never* *ever* returns NULL, making the
checks for it completely unnecessary.
Signed-off-by: Fabiano Fidêncio <fidencio(a)redhat.com>
---
src/interface/interface_backend_netcf.c | 3 +--
src/interface/interface_backend_udev.c | 3 +--
2 files changed, 2 insertions(+), 4 deletions(-)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
4:04 a.m.
New subject: [libvirt] [PATCH 41/42] admin: Use g_autofree on getSocketPath()
Signed-off-by: Fabiano Fidêncio <fidencio(a)redhat.com>
---
src/admin/libvirt-admin.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/src/admin/libvirt-admin.c b/src/admin/libvirt-admin.c
index f156736d9f..d0c191a56a 100644
--- a/src/admin/libvirt-admin.c
+++ b/src/admin/libvirt-admin.c
@@ -99,7 +99,7 @@ virAdmInitialize(void)
static char *
getSocketPath(virURIPtr uri)
{
- char *rundir = virGetUserRuntimeDirectory();
+ g_autofree char *rundir = virGetUserRuntimeDirectory();
char *sock_path = NULL;
size_t i = 0;
@@ -160,7 +160,6 @@ getSocketPath(virURIPtr uri)
}
cleanup:
- VIR_FREE(rundir);
return sock_path;
error:
--
2.24.1
10:11 a.m.
New subject: [libvirt] [PATCH 41/42] admin: Use g_autofree on getSocketPath()
On Thu, Dec 19, 2019 at 11:04:46AM +0100, Fabiano Fidêncio wrote:
Signed-off-by: Fabiano Fidêncio <fidencio(a)redhat.com>
---
src/admin/libvirt-admin.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
s/on/in/ $SUBJECT?
This might be the case for other patches as well.
One note, I would say it's ok to squash some of the patches from this
series together, for example all the g_autofree patches per file for
example.
Pavel
10:22 a.m.
New subject: [libvirt] [PATCH 41/42] admin: Use g_autofree on getSocketPath()
On Thu, Dec 19, 2019 at 5:14 PM Pavel Hrdina <phrdina(a)redhat.com> wrote:
On Thu, Dec 19, 2019 at 11:04:46AM +0100, Fabiano Fidêncio wrote:
> Signed-off-by: Fabiano Fidêncio <fidencio(a)redhat.com>
> ---
> src/admin/libvirt-admin.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
s/on/in/ $SUBJECT?
This might be the case for other patches as well.
Noted.
One note, I would say it's ok to squash some of the patches from this
series together, for example all the g_autofree patches per file for
example.
I really thought about that. However, it may be misleading as I'm not
touching all the possible conversions to use g_autofree in the other
functions.
Pavel
1:33 p.m.
New subject: [libvirt] [PATCH 41/42] admin: Use g_autofree on getSocketPath()
On Thu, Dec 19, 2019 at 05:22:32PM +0100, Fabiano Fidêncio wrote:
On Thu, Dec 19, 2019 at 5:14 PM Pavel Hrdina
<phrdina(a)redhat.com> wrote:
>
> On Thu, Dec 19, 2019 at 11:04:46AM +0100, Fabiano Fidêncio wrote:
> > Signed-off-by: Fabiano Fidêncio <fidencio(a)redhat.com>
> > ---
> > src/admin/libvirt-admin.c | 3 +--
> > 1 file changed, 1 insertion(+), 2 deletions(-)
>
> s/on/in/ $SUBJECT?
>
> This might be the case for other patches as well.
Noted.
>
> One note, I would say it's ok to squash some of the patches from this
> series together, for example all the g_autofree patches per file for
> example.
I really thought about that. However, it may be misleading as I'm not
touching all the possible conversions to use g_autofree in the other
functions.
Well this case is also misleading, since you aren't touching all the
possible g_autofree conversions in this functions
Jano
>
> Pavel
--
libvir-list mailing list
libvir-list(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
4:15 p.m.
New subject: [libvirt] [PATCH 41/42] admin: Use g_autofree on getSocketPath()
On Thu, Dec 19, 2019 at 8:32 PM Ján Tomko <jtomko(a)redhat.com> wrote:
On Thu, Dec 19, 2019 at 05:22:32PM +0100, Fabiano Fidêncio wrote:
>On Thu, Dec 19, 2019 at 5:14 PM Pavel Hrdina <phrdina(a)redhat.com> wrote:
>>
>> On Thu, Dec 19, 2019 at 11:04:46AM +0100, Fabiano Fidêncio wrote:
>> > Signed-off-by: Fabiano Fidêncio <fidencio(a)redhat.com>
>> > ---
>> > src/admin/libvirt-admin.c | 3 +--
>> > 1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> s/on/in/ $SUBJECT?
>>
>> This might be the case for other patches as well.
>
>Noted.
>
>>
>> One note, I would say it's ok to squash some of the patches from this
>> series together, for example all the g_autofree patches per file for
>> example.
>
>I really thought about that. However, it may be misleading as I'm not
>touching all the possible conversions to use g_autofree in the other
>functions.
Well this case is also misleading, since you aren't touching all the
possible g_autofree conversions in this functions
If you're talking specifically about this patch, sock_path is
returned. Meaning that we cannot free it when its out of the scope.
If you caught some other case, please let me know because as I most
likely missed it.
Best Regards,
--
Fabiano Fidêncio
2:14 a.m.
New subject: [libvirt] [PATCH 41/42] admin: Use g_autofree on getSocketPath()
On 12/19/19 8:33 PM, Ján Tomko wrote:
On Thu, Dec 19, 2019 at 05:22:32PM +0100, Fabiano Fidêncio wrote:
> On Thu, Dec 19, 2019 at 5:14 PM Pavel Hrdina <phrdina(a)redhat.com> wrote:
>>
>> On Thu, Dec 19, 2019 at 11:04:46AM +0100, Fabiano Fidêncio wrote:
>> > Signed-off-by: Fabiano Fidêncio <fidencio(a)redhat.com>
>> > ---
>> > src/admin/libvirt-admin.c | 3 +--
>> > 1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> s/on/in/ $SUBJECT?
>>
>> This might be the case for other patches as well.
>
> Noted.
>
>>
>> One note, I would say it's ok to squash some of the patches from this
>> series together, for example all the g_autofree patches per file for
>> example.
>
> I really thought about that. However, it may be misleading as I'm not
> touching all the possible conversions to use g_autofree in the other
> functions.
Well this case is also misleading, since you aren't touching all the
possible g_autofree conversions in this functions
ACK if you squash this in:
diff --git i/src/admin/libvirt-admin.c w/src/admin/libvirt-admin.c
index dcbb8ca84d..4099a54854 100644
--- i/src/admin/libvirt-admin.c
+++ w/src/admin/libvirt-admin.c
@@ -100,11 +100,11 @@ static char *
getSocketPath(virURIPtr uri)
{
g_autofree char *rundir = virGetUserRuntimeDirectory();
- char *sock_path = NULL;
+ g_autofree char *sock_path = NULL;
size_t i = 0;
if (!uri)
- goto cleanup;
+ return NULL;
for (i = 0; i < uri->paramsCount; i++) {
@@ -116,7 +116,7 @@ getSocketPath(virURIPtr uri)
} else {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("Unknown URI parameter '%s'"),
param->name);
- goto error;
+ return NULL;
}
}
@@ -127,7 +127,7 @@ getSocketPath(virURIPtr uri)
if (!uri->scheme) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
"%s", _("No URI scheme specified"));
- goto error;
+ return NULL;
}
if (STREQ(uri->scheme, "libvirtd")) {
legacy = true;
@@ -135,7 +135,7 @@ getSocketPath(virURIPtr uri)
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("Unsupported URI scheme '%s'"),
uri->scheme);
- goto error;
+ return NULL;
}
if (legacy) {
@@ -152,16 +152,11 @@ getSocketPath(virURIPtr uri)
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("Invalid URI path '%s', try
'/system'"),
NULLSTR_EMPTY(uri->path));
- goto error;
+ return NULL;
}
}
- cleanup:
- return sock_path;
-
- error:
- VIR_FREE(sock_path);
- goto cleanup;
+ return g_steal_pointer(&sock_path);
}
Michal
2:22 a.m.
New subject: [libvirt] [PATCH 41/42] admin: Use g_autofree on getSocketPath()
On Fri, Dec 20, 2019 at 9:14 AM Michal Prívozník <mprivozn(a)redhat.com> wrote:
On 12/19/19 8:33 PM, Ján Tomko wrote:
> On Thu, Dec 19, 2019 at 05:22:32PM +0100, Fabiano Fidêncio wrote:
>> On Thu, Dec 19, 2019 at 5:14 PM Pavel Hrdina <phrdina(a)redhat.com> wrote:
>> >>> On Thu, Dec 19, 2019
at 11:04:46AM +0100, Fabiano Fidêncio wrote:
>>> > Signed-off-by: Fabiano Fidêncio <fidencio(a)redhat.com >>> > ---
>>> > src/admin/libvirt-admin.c | 3 +--
>>> > 1 file changed, 1 insertion(+), 2 deletions(-)
>> >>> s/on/in/ $SUBJECT?
>> >>> This might be the case
for other patches as well.
> >> Noted.
> >>
>>> One note, I would say it's ok to squash some of the patches from this
>>> series together, for example all the g_autofree patches per file for
>>> example.
> >> I really thought about that. However, it may be
misleading as I'm not
>> touching all the possible conversions to use g_autofree in the other
>> functions.
> Well this case is also misleading, since you aren't
touching all the
> possible g_autofree conversions in this functions
ACK if you squash this in:
diff --git i/src/admin/libvirt-admin.c w/src/admin/libvirt-admin.c
index dcbb8ca84d..4099a54854 100644
--- i/src/admin/libvirt-admin.c
+++ w/src/admin/libvirt-admin.c
@@ -100,11 +100,11 @@ static char *
getSocketPath(virURIPtr uri)
{
g_autofree char *rundir = virGetUserRuntimeDirectory();
- char *sock_path = NULL;
+ g_autofree char *sock_path = NULL;
size_t i = 0;
if (!uri)
- goto cleanup;
+ return NULL;
for (i = 0; i < uri->paramsCount; i++) {
@@ -116,7 +116,7 @@ getSocketPath(virURIPtr uri)
} else {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("Unknown URI parameter '%s'"),
param->name);
- goto error;
+ return NULL;
}
}
@@ -127,7 +127,7 @@ getSocketPath(virURIPtr uri)
if (!uri->scheme) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
"%s", _("No URI scheme specified"));
- goto error;
+ return NULL;
}
if (STREQ(uri->scheme, "libvirtd")) {
legacy = true;
@@ -135,7 +135,7 @@ getSocketPath(virURIPtr uri)
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("Unsupported URI scheme '%s'"),
uri->scheme);
- goto error;
+ return NULL;
}
if (legacy) {
@@ -152,16 +152,11 @@ getSocketPath(virURIPtr uri)
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("Invalid URI path '%s', try
'/system'"),
NULLSTR_EMPTY(uri->path));
- goto error;
+ return NULL;
}
}
- cleanup:
- return sock_path;
-
- error:
- VIR_FREE(sock_path);
- goto cleanup;
+ return g_steal_pointer(&sock_path);
}
Michal
Thanks, Michal.
I really haven't thought about using g_steal_pointer in this case.
Taking a "review 101", I'd really expect this to be pointed out as:
- If I missed this, it happened most likely because either I'm not
aware of this or I didn't see the opportunity of doing such change;
- Saying something could be changed but not pointing out what makes
the life of **any** developer harder as the developer has to solve a
puzzle in order to understand the comment;
Please, for the next round, consider (and, at this point, you already
know that) that I'm not as smart as the reviewer and point out what
has to be changed. :-)
Michal went one step further and even provided the diff, it's not
needed, but I'd really expect to be pointed to what's wrong during the
review.
I'll go ahead and push the series.
Best Regards,
--
Fabiano Fidêncio
4:04 a.m.
New subject: [libvirt] [PATCH 42/42] admin: Don't check the output of virGetUserRuntimeDirectory()
virGetUserRuntimeDirectory() *never* *ever* returns NULL, making the
checks for it completely unnecessary.
Signed-off-by: Fabiano Fidêncio <fidencio(a)redhat.com>
---
src/admin/libvirt-admin.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/src/admin/libvirt-admin.c b/src/admin/libvirt-admin.c
index d0c191a56a..dcbb8ca84d 100644
--- a/src/admin/libvirt-admin.c
+++ b/src/admin/libvirt-admin.c
@@ -147,9 +147,6 @@ getSocketPath(virURIPtr uri)
if (STREQ_NULLABLE(uri->path, "/system")) {
sock_path = g_strdup_printf(RUNSTATEDIR "/libvirt/%s", sockbase);
} else if (STREQ_NULLABLE(uri->path, "/session")) {
- if (!rundir)
- goto error;
-
sock_path = g_strdup_printf("%s/%s", rundir, sockbase);
} else {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
--
2.24.1
1:33 p.m.
New subject: [libvirt] [PATCH 42/42] admin: Don't check the output of virGetUserRuntimeDirectory()
On Thu, Dec 19, 2019 at 11:04:47AM +0100, Fabiano Fidêncio wrote:
virGetUserRuntimeDirectory() *never* *ever* returns NULL, making the
checks for it completely unnecessary.
Signed-off-by: Fabiano Fidêncio <fidencio(a)redhat.com>
---
src/admin/libvirt-admin.c | 3 ---
1 file changed, 3 deletions(-)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
1830
days inactive
1831
days old
100 comments
5 participants
participants (5)
-
Daniel Henrique Barboza -
Fabiano Fidêncio -
Ján Tomko -
Michal Prívozník -
Pavel Hrdina