Subject: Re: Patch to ticket 228

Re: Patch to ticket 228

From: Jernej Kovacic <jkovacic_at_gmail.com>
Date: Sat, 19 Nov 2011 15:21:09 +0100

Hi,

as there were no comments I have concluded that my addition to the
manpage is OK. Please find the patch in the attachment.

Regards,
Jernej

On Mon, Nov 14, 2011 at 10:50 PM, Jernej Kovacic <jkovacic_at_gmail.com> wrote:
> Hello,
>
> finally found some time. Let me reply to some remarks first:
>
>> I would favor using ** and taking a length parameter, so that we work
>> toward less malloc within libssh2 instead of more
>
> That was my initial proposal but then it was agreed that the function
> should allocate memory itself.
>
>> Can there really be NULL names within method lists? I do not think so.
>> kex.c has many loops over these method lists and none skips over NULL
>> names like done here. Some ends on NULL names, some assumes name is
>> alway valid.
>
> It's perfect if it really never occurs but it never hurts to recheck
> it. It's just my "defensive" (or you may call it "paranoid")
> developing approach that I got used to.  In this case, if something
> "suspicious" or meaningless is found, it should not be returned it as
> it may confuse or even crash a poorly written application.
>
> Alternatively, the function could return an error if something like
> this is detected.
>
>> I only have one comment left here: 'char ***arg' is a really unfortunate argument type. I bet that will scare weak-hearted users really good. The least we can do is to offer an example in the man page showing how to use it...
>
> Unfortunately this is a case if a function must return an allocated
> array of "strings". If the number of asterisks is really such a
> problem, they could be "hidden" by wrapping the list into a one-member
> struct. But in my opinion this really doesn't make any sense. Instead
> I have prepared a short example to be inserted into the man page.
> Please review it, let me know about any comments about its style
> (khmm, are there any Nroff tags to mark an example of code?), should
> something be added or cut? Then I will prepare and send the patch.
>
> ------------ sample start -------------------
> .SH EXAMPLE
> #include "libssh2.h"
>
> const char **algorithms;
> int rc;
> LIBSSH2_SESSION *session;
>
> /* initilize session */
> session = libssh2_session_init();
> rc = libssh2_session_supported_algs(session,
>                                     LIBSSH2_METHOD_CRYPT_CS,
>                                    &algorithms);
> if (rc>0) {
>    /* the call succeeded, do sth. with the list of algorithms,
>       (e.g. list them)... */
>    int i;
>    printf("Supported symmetric algorithms:\n");
>    for ( i=0; i<rc; i++ )
>        printf("\t%s\n", algorithms[i]);
>
>    /* ... and free the allocated memory when not needed anymore */
>    libssh2_free(session, algorithms);
> }
> else {
>    /* call failed, error handling */
> }
> ---------------- sample end ---------------------
>
> Regards, Jernej
>

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

Received on 2011-11-19