On 10/23/2015 01:43 PM, Peter Stuge wrote:
Salvador,
Salvador Fandino wrote:
I converted it into "_libssh2_error_flags" which accepts a flag
indicating whether the given string must be duplicated
Please think about how and where allocated memory is being freed.
Try creating and destroying sessions in a loop - there is a leak.
Do you need to free this memory in other places besides that?
Is there a LIBSSH2_REALLOC?
Style: the function takes several parameters whose names start with err.
You add a new parameter named flags, which does not start with err.
Please be careful to always follow style wherever you make changes.
Name the new parameter e.g. errflags.
Ok, lets try again:
Now "err_msg" is also freed from "session_free" when the DUP flag is set.
The "flags" argument to "_libssh2_error_flags" has been renamed to
"errflags".
>From 5bf87c70453be606853001e9f5a10d9d75d6ea29 Mon Sep 17 00:00:00 2001
From: Salvador Fandino <sfand...@yahoo.com>
Date: Wed, 21 Oct 2015 15:03:02 +0200
Subject: [PATCH 1/2] Support allocating the error message on the heap
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Before this patch "_libssh2_error" required the error message to be a
static string.
This patch adds a new function "_libssh2_error_flags" accepting an
additional "flags" argument and specifically the flag
"LIBSSH2_ERR_FLAG_DUP" indicating that the passed string must be
duplicated into the heap.
Then, the method "_libssh2_error" has been rewritten to use that new
function under the hood.
Signed-off-by: Salvador Fandino <sfand...@yahoo.com>
Signed-off-by: Salvador Fandiño <sfand...@yahoo.com>
---
src/libssh2_priv.h | 5 +++++
src/misc.c | 28 ++++++++++++++++++++++++++--
src/misc.h | 1 +
src/session.c | 5 +++++
4 files changed, 37 insertions(+), 2 deletions(-)
diff --git a/src/libssh2_priv.h b/src/libssh2_priv.h
index 78d4ced..1023943 100644
--- a/src/libssh2_priv.h
+++ b/src/libssh2_priv.h
@@ -631,6 +631,7 @@ struct _LIBSSH2_SESSION
/* Error tracking */
const char *err_msg;
int err_code;
+ int err_flags;
/* struct members for packet-level reading */
struct transportpacket packet;
@@ -950,6 +951,10 @@ _libssh2_debug(LIBSSH2_SESSION * session, int context, const char *format, ...)
/* Something very bad is going on */
#define LIBSSH2_MAC_INVALID -1
+/* Flags for _libssh2_error_flags */
+/* Error message is allocated on the heap */
+#define LIBSSH2_ERR_FLAG_DUP 1
+
/* SSH Packet Types -- Defined by internet draft */
/* Transport Layer */
#define SSH_MSG_DISCONNECT 1
diff --git a/src/misc.c b/src/misc.c
index 283daea..320df44 100644
--- a/src/misc.c
+++ b/src/misc.c
@@ -51,10 +51,29 @@
#include <stdio.h>
#include <errno.h>
-int _libssh2_error(LIBSSH2_SESSION* session, int errcode, const char* errmsg)
+int _libssh2_error_flags(LIBSSH2_SESSION* session, int errcode, const char* errmsg, int errflags)
{
- session->err_msg = errmsg;
+ if (session->err_flags & LIBSSH2_ERR_FLAG_DUP)
+ LIBSSH2_FREE(session, (char *)session->err_msg);
+
session->err_code = errcode;
+ session->err_flags = 0;
+
+ if ((errmsg != NULL) && ((errflags & LIBSSH2_ERR_FLAG_DUP) != 0)) {
+ size_t len = strlen(errmsg);
+ char *copy = LIBSSH2_ALLOC(session, len + 1);
+ if (copy) {
+ memcpy(copy, errmsg, len + 1);
+ session->err_flags = LIBSSH2_ERR_FLAG_DUP;
+ session->err_msg = copy;
+ }
+ else
+ /* Out of memory: this code path is very unlikely */
+ session->err_msg = "former error forgotten (OOM)";
+ }
+ else
+ session->err_msg = errmsg;
+
#ifdef LIBSSH2DEBUG
if((errcode == LIBSSH2_ERROR_EAGAIN) && !session->api_block_mode)
/* if this is EAGAIN and we're in non-blocking mode, don't generate
@@ -67,6 +86,11 @@ int _libssh2_error(LIBSSH2_SESSION* session, int errcode, const char* errmsg)
return errcode;
}
+int _libssh2_error(LIBSSH2_SESSION* session, int errcode, const char* errmsg)
+{
+ return _libssh2_error_flags(session, errcode, errmsg, 0);
+}
+
#ifdef WIN32
static int wsa2errno(void)
{
diff --git a/src/misc.h b/src/misc.h
index f99b773..54ae546 100644
--- a/src/misc.h
+++ b/src/misc.h
@@ -49,6 +49,7 @@ struct list_node {
struct list_head *head;
};
+int _libssh2_error_flags(LIBSSH2_SESSION* session, int errcode, const char* errmsg, int errflags);
int _libssh2_error(LIBSSH2_SESSION* session, int errcode, const char* errmsg);
void _libssh2_list_init(struct list_head *head);
diff --git a/src/session.c b/src/session.c
index 9e9343f..cc77a7a 100644
--- a/src/session.c
+++ b/src/session.c
@@ -1058,6 +1058,11 @@ session_free(LIBSSH2_SESSION *session)
LIBSSH2_FREE(session, session->server_hostkey);
}
+ /* error string */
+ if (session->err_msg && ((session->err_flags & LIBSSH2_ERR_FLAG_DUP) != 0)) {
+ LIBSSH2_FREE(session, (char *)session->err_msg);
+ }
+
LIBSSH2_FREE(session, session);
return 0;
--
2.5.0
>From ee7bda6b0e810cad9d8855da7fc54f6099370e3f Mon Sep 17 00:00:00 2001
From: Salvador Fandino <sfand...@yahoo.com>
Date: Thu, 15 Oct 2015 17:36:03 +0200
Subject: [PATCH 2/2] Add function libssh2_session_set_last_error
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Net::SSH2, the Perl wrapping module for libssh2 implements several features*
on top of libssh2 that can fail and so need some mechanism to report the error
condition to the user.
Until now, besides the error state maintained internally by libssh2, another
error state was maintained at the Perl level for every session object and then
additional logic was used to merge both error states. That is a maintenance
nighmare, and actually there is no way to do it correctly and consistently.
In order to allow the high level language to add new features to the library
but still rely in its error reporting features the new function
libssh2_session_set_last_error (that just exposses _libssh2_error_flags) is
introduced.
*) For instance, connecting to a remote SSH service giving the hostname and
port.
Signed-off-by: Salvador Fandino <sfand...@yahoo.com>
Signed-off-by: Salvador Fandiño <sfand...@yahoo.com>
---
include/libssh2.h | 3 +++
src/session.c | 19 ++++++++++++++++++-
2 files changed, 21 insertions(+), 1 deletion(-)
diff --git a/include/libssh2.h b/include/libssh2.h
index 8b4695a..f3d976c 100644
--- a/include/libssh2.h
+++ b/include/libssh2.h
@@ -567,6 +567,9 @@ LIBSSH2_API int libssh2_session_last_error(LIBSSH2_SESSION *session,
char **errmsg,
int *errmsg_len, int want_buf);
LIBSSH2_API int libssh2_session_last_errno(LIBSSH2_SESSION *session);
+LIBSSH2_API int libssh2_session_set_last_error(LIBSSH2_SESSION* session,
+ int errcode,
+ const char* errmsg);
LIBSSH2_API int libssh2_session_block_directions(LIBSSH2_SESSION *session);
LIBSSH2_API int libssh2_session_flag(LIBSSH2_SESSION *session, int flag,
diff --git a/src/session.c b/src/session.c
index cc77a7a..06e61dd 100644
--- a/src/session.c
+++ b/src/session.c
@@ -1290,7 +1290,24 @@ libssh2_session_last_errno(LIBSSH2_SESSION * session)
return session->err_code;
}
-/* libssh2_session_flag
+/* libssh2_session_set_last_error
+ *
+ * Sets the internal error code for the session.
+ *
+ * This function is available specifically to be used by high level
+ * language wrappers (i.e. Python or Perl) that may extend the library
+ * features while still relying on its error reporting mechanism.
+ */
+LIBSSH2_API int
+libssh2_session_set_last_error(LIBSSH2_SESSION* session,
+ int errcode,
+ const char* errmsg)
+{
+ return _libssh2_error_flags(session, errcode, errmsg,
+ LIBSSH2_ERR_FLAG_DUP);
+}
+
+/* Libssh2_session_flag
*
* Set/Get session flags
*
--
2.5.0
_______________________________________________
libssh2-devel http://cool.haxx.se/cgi-bin/mailman/listinfo/libssh2-devel
