Cleanups after adopting g_get_*_dir()

[libvirt] [PATCH 16/42] rpc: Don't check the output of virGetUserConfigDirectory()

Posted by Fabiano Fidêncio 6 years, 1 month ago

virGetUserConfigDirectory() *never* *ever* returns NULL, making the
checks for it completely unnecessary.

Signed-off-by: Fabiano Fidêncio <fidencio@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

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 16/42] rpc: Don't check the output of virGetUserConfigDirectory()

Posted by Ján Tomko 6 years, 1 month ago

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@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@redhat.com>

Jano
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 16/42] rpc: Don't check the output of virGetUserConfigDirectory()

Posted by Fabiano Fidêncio 6 years, 1 month ago

On Thu, Dec 19, 2019 at 8:23 PM Ján Tomko <jtomko@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@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


--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list