Re: [libvirt] [PATCH] rpc: fix handling of SSH auth failure code

On Fri, 2018-08-31 at 11:15 +0100, Daniel P. Berrangé wrote: > The result of libssh2_userauth_password is being assigned to 'ret' in > one branch and 'rc' in the other branch. Checks are all done against the > 'ret' variable, so one branch never does the correct check. > > Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com&gt; > --- > src/rpc/virnetsshsession.c | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) The fix itself is correct, so: Reviewed-by: Andrea Bolognani <abologna(a)redhat.com&gt; However, see below. > /* determine exist status */ > - if (ret == LIBSSH2_ERROR_AUTHENTICATION_FAILED) > + if (rc == LIBSSH2_ERROR_AUTHENTICATION_FAILED) > return 1; > else > return -1; Since we have a cleanup label right after this, it would make sense to change both 'return' to 'ret = ': that would ensure the function has a single return and also prevent 'password' from leaking when an error which is not AUTHENTICATION_FAILED is returned by libssh2_userauth_password().