| Message ID | 1399996972-23429-6-git-send-email-armbru@redhat.com |
|---|---|
| State | New |
| Headers | show |
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" <rjones@redhat.com> > Signed-off-by: Markus Armbruster <armbru@redhat.com> > --- > 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. Rich.
"Richard W.M. Jones" <rjones@redhat.com> 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" <rjones@redhat.com> >> Signed-off-by: Markus Armbruster <armbru@redhat.com> >> --- >> 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.
On Wed, May 14, 2014 at 01:06:21PM +0200, Markus Armbruster wrote: > "Richard W.M. Jones" <rjones@redhat.com> 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" <rjones@redhat.com> > >> Signed-off-by: Markus Armbruster <armbru@redhat.com> > >> --- > >> 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? Rich.
"Richard W.M. Jones" <rjones@redhat.com> writes: > On Wed, May 14, 2014 at 01:06:21PM +0200, Markus Armbruster wrote: >> "Richard W.M. Jones" <rjones@redhat.com> 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" <rjones@redhat.com> >> >> Signed-off-by: Markus Armbruster <armbru@redhat.com> >> >> --- >> >> 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 :)
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" <rjones@redhat.com> > Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Richard W.M. Jones <rjones@redhat.com> > 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); > > -- > 1.8.1.4
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);
libssh2_session_last_error() already returns the error code. Cc: "Richard W.M. Jones" <rjones@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com> --- block/ssh.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-)