[libvirt] [PATCH 0/3] remote: misc cleanups to client driver

A few bits of cleanup identified during work on the daemon splitting patches sent now since they are independent. Daniel P. Berrangé (3): remote: stop declaring variables in the middle of a function remote: use autofree for many string variables remote: conditionally declare tty variable for non-Win32 platforms src/remote/remote_driver.c | 61 ++++++++++++++------------------------ 1 file changed, 23 insertions(+), 38 deletions(-) -- 2.21.0

The doRemoteOpen method was a little unusual in declaring a bunch of local variables in the middle of the function. Move them to the top as it is normal libvirt style. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/remote/remote_driver.c | 28 ++++++++++++++++------------ 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index df58b23c8c..eb128a87a6 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -789,6 +789,22 @@ doRemoteOpen(virConnectPtr conn, char *daemonPath = NULL; #endif char *tls_priority = NULL; + char *name = NULL; + char *command = NULL; + char *sockname = NULL; + char *netcat = NULL; + char *port = NULL; + char *authtype = NULL; + char *username = NULL; + char *pkipath = NULL; + char *keyfile = NULL; + char *sshauth = NULL; + char *knownHostsVerify = NULL; + char *knownHosts = NULL; + bool sanity = true; + bool verify = true; + bool tty ATTRIBUTE_UNUSED = true; + int retcode = VIR_DRV_OPEN_ERROR; /* We handle *ALL* URIs here. The caller has rejected any * URIs we don't care about */ @@ -848,18 +864,6 @@ doRemoteOpen(virConnectPtr conn, return VIR_DRV_OPEN_ERROR; } - /* Local variables which we will initialize. These can - * get freed in the failed: path. - */ - char *name = NULL, *command = NULL, *sockname = NULL, *netcat = NULL; - char *port = NULL, *authtype = NULL, *username = NULL; - bool sanity = true, verify = true, tty ATTRIBUTE_UNUSED = true; - char *pkipath = NULL, *keyfile = NULL, *sshauth = NULL; - - char *knownHostsVerify = NULL, *knownHosts = NULL; - - /* Return code from this function, and the private data. */ - int retcode = VIR_DRV_OPEN_ERROR; /* Remote server defaults to "localhost" if not specified. */ if (conn->uri && conn->uri->port != 0) { -- 2.21.0

Simplify the clean code paths for doRemoteOpen by using VIR_AUTOFREE Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/remote/remote_driver.c | 55 +++++++++++--------------------------- 1 file changed, 16 insertions(+), 39 deletions(-) diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index eb128a87a6..aae1976008 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -786,25 +786,24 @@ doRemoteOpen(virConnectPtr conn, trans_libssh, } transport; #ifndef WIN32 - char *daemonPath = NULL; + VIR_AUTOFREE(char *) daemonPath = NULL; #endif - char *tls_priority = NULL; - char *name = NULL; - char *command = NULL; - char *sockname = NULL; - char *netcat = NULL; - char *port = NULL; - char *authtype = NULL; - char *username = NULL; - char *pkipath = NULL; - char *keyfile = NULL; - char *sshauth = NULL; - char *knownHostsVerify = NULL; - char *knownHosts = NULL; + VIR_AUTOFREE(char *) tls_priority = NULL; + VIR_AUTOFREE(char *) name = NULL; + VIR_AUTOFREE(char *) command = NULL; + VIR_AUTOFREE(char *) sockname = NULL; + VIR_AUTOFREE(char *) netcat = NULL; + VIR_AUTOFREE(char *) port = NULL; + VIR_AUTOFREE(char *) authtype = NULL; + VIR_AUTOFREE(char *) username = NULL; + VIR_AUTOFREE(char *) pkipath = NULL; + VIR_AUTOFREE(char *) keyfile = NULL; + VIR_AUTOFREE(char *) sshauth = NULL; + VIR_AUTOFREE(char *) knownHostsVerify = NULL; + VIR_AUTOFREE(char *) knownHosts = NULL; bool sanity = true; bool verify = true; bool tty ATTRIBUTE_UNUSED = true; - int retcode = VIR_DRV_OPEN_ERROR; /* We handle *ALL* URIs here. The caller has rejected any * URIs we don't care about */ @@ -1257,29 +1256,7 @@ doRemoteOpen(virConnectPtr conn, "by the remote side."); } - /* Successful. */ - retcode = VIR_DRV_OPEN_SUCCESS; - - cleanup: - /* Free up the URL and strings. */ - VIR_FREE(name); - VIR_FREE(command); - VIR_FREE(sockname); - VIR_FREE(authtype); - VIR_FREE(netcat); - VIR_FREE(sshauth); - VIR_FREE(keyfile); - VIR_FREE(username); - VIR_FREE(port); - VIR_FREE(pkipath); - VIR_FREE(tls_priority); - VIR_FREE(knownHostsVerify); - VIR_FREE(knownHosts); -#ifndef WIN32 - VIR_FREE(daemonPath); -#endif - - return retcode; + return VIR_DRV_OPEN_SUCCESS; failed: virObjectUnref(priv->remoteProgram); @@ -1296,7 +1273,7 @@ doRemoteOpen(virConnectPtr conn, #endif VIR_FREE(priv->hostname); - goto cleanup; + return VIR_DRV_OPEN_ERROR; } #undef EXTRACT_URI_ARG_STR #undef EXTRACT_URI_ARG_BOOL -- 2.21.0

The 'tty' variable is only used on Win32. Instead of just annotating it with ATTRIBUTE_UNUSED, make its declaration conditional on WIN32 so that it is clear why it is not used. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/remote/remote_driver.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index aae1976008..acc6a87bb3 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -803,7 +803,9 @@ doRemoteOpen(virConnectPtr conn, VIR_AUTOFREE(char *) knownHosts = NULL; bool sanity = true; bool verify = true; - bool tty ATTRIBUTE_UNUSED = true; +#ifndef WIN32 + bool tty = true; +#endif /* We handle *ALL* URIs here. The caller has rejected any * URIs we don't care about */ @@ -908,7 +910,9 @@ doRemoteOpen(virConnectPtr conn, EXTRACT_URI_ARG_BOOL("no_sanity", sanity); EXTRACT_URI_ARG_BOOL("no_verify", verify); +#ifndef WIN32 EXTRACT_URI_ARG_BOOL("no_tty", tty); +#endif if (STRCASEEQ(var->name, "authfile")) { /* Strip this param, used by virauth.c */ -- 2.21.0

On Fri, Jul 5, 2019 at 11:01 AM Daniel P. Berrangé <berrange@redhat.com> wrote:
A few bits of cleanup identified during work on the daemon splitting patches sent now since they are independent.
Reviewed-by: Fabiano Fidêncio <fidencio@redhat.com>
Daniel P. Berrangé (3): remote: stop declaring variables in the middle of a function remote: use autofree for many string variables remote: conditionally declare tty variable for non-Win32 platforms
src/remote/remote_driver.c | 61 ++++++++++++++------------------------ 1 file changed, 23 insertions(+), 38 deletions(-)
-- 2.21.0
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
participants (2)
-
Daniel P. Berrangé
-
Fabiano Fidêncio