Subject: Re: libssh2 master 942a40b... Call libssh2_error for every knownhost API failure.

Re: libssh2 master 942a40b... Call libssh2_error for every knownhost API failure.

From: Peter Stuge <peter_at_stuge.se>
Date: Fri, 26 Feb 2010 19:26:55 +0100

Overall this is a great improvement - thanks Alex!

> commit 942a40b48250c92eeb0cf620ded6e943a4de025a
..

> @@ -593,8 +638,11 @@ libssh2_knownhost_readline(LIBSSH2_KNOWNHOSTS *hosts,
> }
>
> if(!*cp || !len)
> - /* illegal line */
> - return LIBSSH2_ERROR_METHOD_NOT_SUPPORTED;
> + /* illegal line */ {
> + return libssh2_error(hosts->session,
> + LIBSSH2_ERROR_METHOD_NOT_SUPPORTED,
> + "Failed to parse known_hosts line", 0);
> + }

I'm unhappy with the proliferation of braces for single-statement
cases. The comment above is also not to well placed IMO, but I find
the many braces added in this commit to be a worse eyesore. :\
Is there a coding style rule for this or simply your preference?

> +++ b/src/misc.c
> @@ -49,6 +49,42 @@
>
> #include <errno.h>
>
> +#ifdef LIBSSH2DEBUG
> +
> +int libssh2_error(LIBSSH2_SESSION* session, int errcode, char* errmsg,
> + int should_free)
> +{
> + if (session->err_msg && session->err_should_free) {
> + LIBSSH2_FREE(session, session->err_msg);
> + }
> + session->err_msg = errmsg;
> + session->err_msglen = strlen(errmsg);
> + session->err_should_free = should_free;
> + session->err_code = errcode;
> + _libssh2_debug(session, LIBSSH2_TRACE_ERROR, "%d - %s", session->err_code,
> + session->err_msg);
> +
> + return errcode;
> +}
> +
> +#else /* ! LIBSSH2DEBUG */
> +
> +int libssh2_error(LIBSSH2_SESSION* session, int errcode, char* errmsg,
> + int should_free)
> +{
> + if (session->err_msg && session->err_should_free) {
> + LIBSSH2_FREE(session, session->err_msg);
> + }
> + session->err_msg = errmsg;
> + session->err_msglen = strlen(errmsg);
> + session->err_should_free = should_free;
> + session->err_code = errcode;
> +
> + return errcode;
> +}
> +
> +#endif /* ! LIBSSH2DEBUG */

The above is very redundant. Please fix it so #ifdef only guards the
_debug call inside a single copy of the function.

//Peter
_______________________________________________
libssh2-devel http://cool.haxx.se/cgi-bin/mailman/listinfo/libssh2-devel
Received on 2010-02-26