Subject: Re: [PATCH] Support for exit-signal messages

Re: [PATCH] Support for exit-signal messages

From: Simon Josefsson <simon_at_josefsson.org>
Date: Sun, 03 Oct 2010 21:50:04 +0200

Tommy Lindgren <tommy.lindgren_at_gmail.com> writes:

> Any opinion on how that modification should look? Personally I think it
> looks nicer with a separate function for retrieving the error message
> (and language tag). But if we are extending the new function I guess the
> signature could look something like
>
> LIBSSH2_API const char *
> libssh2_channel_get_exit_signal(LIBSSH2_CHANNEL *channel, char *errmsg,
> char **langtag)
>
> or possibly
>
> LIBSSH2_API const char *
> libssh2_channel_get_exit_signal(LIBSSH2_CHANNEL *channel, char *errmsg,
> int *errmsg_len, char **langtag, int *langtag_len)
>
> Thoughts?

Generally I think it is a bad idea for functions to return strings --
there is no way to do useful error handling, since there is only one
type of error (NULL) which often is insufficient to describe what the
problem is.

Thus, I would suggest something like this:

/* Get exit signal (without leading "SIG"), error message, and language
   tag into newly allocated buffers of indicated length. Caller can
   use NULL pointers to indicate that the value should not be set. The
   *_len variables are set if they are non-NULL even if the
   corresponding string parameter is NULL. Returns LIBSSH2_ERROR_NONE
   on success, or an API error code. */
LIBSSH2_API int
libssh2_channel_get_exit_signal(LIBSSH2_CHANNEL *channel,
                                char **exitsignal,
                                size_t *exitsignal_len
                                char **errmsg,
                                size_t *errmsg_len,
                                char **langtag,
                                size_t *langtag_len)

There is precedent for libssh2 functions to allocate memory (e.g.,
libssh2_base64_decode), but we could also consider an interface that
doesn't allocate memory. I find that alternative more difficult to use
from an application, though.

/Simon
_______________________________________________
libssh2-devel http://cool.haxx.se/cgi-bin/mailman/listinfo/libssh2-devel
Received on 2010-10-03