Subject: Re: Patch to ticket 228

Re: Patch to ticket 228

From: Alexander Lamaison <swish_at_lammy.co.uk>
Date: Mon, 19 Sep 2011 22:40:59 +0100

On 19 September 2011 21:56, Jernej Kovacic <jkovacic_at_gmail.com> wrote:

> diff --git a/docs/libssh2_session_supported_algs.3 b/docs/libssh2_session_supported_algs.3
> new file mode 100644
> index 0000000..8b8ffd0
> --- /dev/null
> +++ b/docs/libssh2_session_supported_algs.3
> @@ -0,0 +1,29 @@
> +.TH libssh2_session_supported_algs 3 "19 Sep 2011" "libssh2 1.4.0" "libssh2 manual"

The function is called libssh2_session_supported_algs but it doesn't
actually take a session argument. I'm not familiar with what this
function is designed to do. How is it's task independent of the
session?

...
> +int libssh2_session_supported_algs(int method_type, char** algs, unsigned int nalgs);
> +.SH DESCRIPTION
> +Get a list of supported algorithms for each method_type. The method_type parameter is equivalent
> +to method_type in \fIlibssh2_session_method_pref(3)\fP. Algs must be user preallocated array of pointers to strings
> +(e.g. char* methods[N_METHODS]) and nalgs is a number of preallocated elemnts in this array.
> +If successful, the function will fill this array with supported algorithms (the same names as defined in RFC 4253)
> +and terminate it with a NULL pointer.

Who is responsible for freeing the strings added to the array? How
does the caller know how big to make the array?

> +.SH RETURN VALUE
> +On success, a number of returned algorithms (i.e a positive number will be returned).

What number? The number added to the array? Is this not fixed? If
not, why does the function not allocate the array itself so that it's
the right size?

...
> --- a/include/libssh2.h
> +++ b/include/libssh2.h
> +/*
> + Retrieve a list of supported algorithms for given method_type (see ibssh2_session_method_pref)
> + algs - user preallocated array of pointers to string which will be filled (and NULL terminated) in by the function
> + nalgs - number of reserved elements in algs, incl. the NULL termination

Erm ... why would nalgs be NULL terminated? It's an array of pointers
and its size is passed around explicitly.

> +LIBSSH2_API int libssh2_session_supported_algs(int method_type, char** algs, unsigned int nalgs);

We should use size_t for all new APIs. unsigned int only exists in
the current API to maintain ABI compatability. They will also be
changed to use size_t at the next soname bump.

Alex

--
Swish - Easy SFTP for Windows Explorer (http://www.swish-sftp.org)
_______________________________________________
libssh2-devel http://cool.haxx.se/cgi-bin/mailman/listinfo/libssh2-devel
Received on 2011-09-19