www.libssh2.org | Daily snapshots | Mailing list archive | Docs | Examples | github

Archive Index This month's Index

Subject: Re: libssh2 MorphOS patch

Re: libssh2 MorphOS patch

From: Daniel Stenberg <daniel_at_haxx.se>
Date: Thu, 2 Aug 2012 00:00:38 +0200 (CEST)

On Wed, 1 Aug 2012, Leif Salomonsson wrote:

Thanks for your work!

> Because of collissions with bsdsocket send() and recv() functions, some
> changes where made in libssh2_priv.h and session.c, where these names where
> used for callbacks variables in LIBSSH2_SESSION.

I don't get why those "collissions" cause any problems. Do you have them as
macros/defines or something? Why do they collide? Every single operating
system we ever build libssh2 on have send, recv etc...

Your changes indicate there is something seriously wrong with your headers.

Further, your indents and white space changes were off. We use indent-level as
4 spaces and no tabs anywhere.

> Similar issue in agent.c, where ->connect() is changed to ->_connect_().

Perhaps, but you also changed the *actual* connect() call to _connect_() which
surely doesn't work?

> Some blocking mode set/get changes/fixes in session.c.

I don't like how everything MorphOS there is within HAVE_IOCTLSOCKET_CASE when
they obviously are far from related to that function alone. For example:

+#include <proto/socket.h>

... should rather be:

#include <proto/socket.h>

And this:

+#if defined(HAVE_IOCTLSOCKET_CASE) && (GETBLOCK == 0)
+ /* Amiga/MorphOS */
+ unsigned int option_value;
+ unsigned int option_len = sizeof(option_value);
+ if (getsockopt(sockfd, SOL_SOCKET, SO_ERROR, (void *) &option_value,
&option_len)) {
+ /* Assume blocking on error */
+ return 1;
+ }

... I think should rather use #ifdef _MORPHOS or something as it seems more
related to the OS rather than the actual way ioctlsocket() is used.

BTW, MorphOS is not present here and I'd appreciate if you'd add it:

  / daniel.haxx.se
libssh2-devel http://cool.haxx.se/cgi-bin/mailman/listinfo/libssh2-devel
Received on 2012-08-02

the libssh2 team