Hi,

Dave McCaldon wrote:
> I was hoping to get my patch from Nov. 4th into the code base:
> 
> [PATCH] Map SSH_MSG_USERAUTH_FAILURE to LIBSSH2_ERROR_LOGIN_FAILED
> in libssh2_userauth_password()

Thanks for the reminder. When I first saw this my immediate reaction
was that it was an improvement, but upon closer review I hesitate a
little to commit it.


> What's the correct process for submitting this kind of thing?

I think you did the right thing sending it to the list. But please
take care to send individual patches for logically grouped changes
rather than squashed commits. (You may want to squash and that's
fine, but please do it transparently before sending out patches.)

Also, note that the first line of the commit message is used as
filename, as a single line description of your change in the git log,
and in the shortlog on gitweb. Try to write a very concise
description on the first line, not longer than 72 chars or so, then
leave one blank line, and explain your commit further there.
(For a change such as this, I could just as well have fixed this up
when committing, but you can save me that time. :) A more involved
change might need better understanding of the changed code, so it
would not be as easy for a committer to "fix up" the log message.


Dave McCaldon wrote:
> The patch adds a new error code, LIBSSH2_ERROR_LOGIN_FAILED which
> is returned in the case of SSH_MSG_USERAUTH_FAILURE (this is the
> same as the logic in libssh2_userauth_publickey_fromfile() that
> handles an unrecognized public key).

LOGIN_FAILED is very generic. The userauth_publickey function uses
a very specific PUBLICKEY_UNVERIFIED return code for the same SSH
layer error. I think that either the now suggested LOGIN_FAILED
should be renamed to PASSWORD_NOT_ACCEPTED or similar, or both of
these error branches should just return one and the same
LIBSSH2_ERROR_ to the application - after all it is the same error
code in the SSH protocol, and the application knows which function
it just called. I don't see a big benefit with dividing the error
code inside libssh2?


//Peter
_______________________________________________
libssh2-devel http://cool.haxx.se/cgi-bin/mailman/listinfo/libssh2-devel

Reply via email to