Re: [Qemu-devel] [PATCH 05/18] block/ssh: Drop superfluous libssh2_session_last_errno() calls

[Markus Armbruster]

"Richard W.M. Jones" <address@hidden> writes:

> On Wed, May 14, 2014 at 01:06:21PM +0200, Markus Armbruster wrote:
>> "Richard W.M. Jones" <address@hidden> writes:
>> 
>> > On Tue, May 13, 2014 at 06:02:39PM +0200, Markus Armbruster wrote:
>> >> libssh2_session_last_error() already returns the error code.
>> >> 
>> >> Cc: "Richard W.M. Jones" <address@hidden>
>> >> Signed-off-by: Markus Armbruster <address@hidden>
>> >> ---
>> >>  block/ssh.c | 9 ++++-----
>> >>  1 file changed, 4 insertions(+), 5 deletions(-)
>> >> 
>> >> diff --git a/block/ssh.c b/block/ssh.c
>> >> index aa63c9d..e38d232 100644
>> >> --- a/block/ssh.c
>> >> +++ b/block/ssh.c
>> >> @@ -121,10 +121,9 @@ session_error_report(BDRVSSHState *s, const char 
>> >> *fs, ...)
>> >>          char *ssh_err;
>> >>          int ssh_err_code;
>> >>  
>> >> -        libssh2_session_last_error((s)->session, &ssh_err, NULL, 0);
>> >>          /* This is not an errno.  See <libssh2.h>. */
>> >> -        ssh_err_code = libssh2_session_last_errno((s)->session);
>> >> -
>> >> +        ssh_err_code = libssh2_session_last_error(s->session,
>> >> +                                                  &ssh_err, NULL, 0);
>> >>          error_printf(": %s (libssh2 error code: %d)", ssh_err, 
>> >> ssh_err_code);
>> >>      }
>> >>  
>> >> @@ -145,9 +144,9 @@ sftp_error_report(BDRVSSHState *s, const char *fs, 
>> >> ...)
>> >>          int ssh_err_code;
>> >>          unsigned long sftp_err_code;
>> >>  
>> >> -        libssh2_session_last_error((s)->session, &ssh_err, NULL, 0);
>> >>          /* This is not an errno.  See <libssh2.h>. */
>> >> -        ssh_err_code = libssh2_session_last_errno((s)->session);
>> >> +        ssh_err_code = libssh2_session_last_error(s->session,
>> >> +                                                  &ssh_err, NULL, 0);
>> >>          /* See <libssh2_sftp.h>. */
>> >>          sftp_err_code = libssh2_sftp_last_error((s)->sftp);
>> >
>> > Yes, I'm not quite sure what was happening here.  I checked the source
>> > of libssh2 and as you say, libssh2_session_last_error returns the
>> > error code, so there is no need to call libssh2_session_last_errno as
>> > well.
>> >
>> > Therefore, ACK.
>> 
>> Did you review the patch?  If yes, I'd like to convert your ACK to a
>> formal Reviewed-by.  Same for PATCH 06-09.
>
> Yes, I applied all 5 of them to qemu.git, compiled it and also checked
> over the patches.
>
> So ACK to all of them.
>
> Do I need to do something else?

We're pretty formal about commit tags on this list.

We interpret "Acked-by" conservatively.  Something like "this patch
touches stuff I'm responsible for, I'm aware of the patch, and I don't
object to it".

To say "I reviewed this patch, and I think it's ready for commit", we
use "Reviewed-by".

"ACK" is neither, so I asked for clarification :)