headers
Leif Salomonsson | 1 Aug 2012 20:21

libssh2 MorphOS patch

Hello,

I ported libssh2 to MorphOS (http://www.morphos-team.net).
I would like to have changes merged with main tree if possible.
Patch file attached. Here is a description of the changes:

-----------

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.

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

Again, libssh2_priv.h: struct _LIBSSH2_CRYPT_METHOD has a function
named "crypt", renamed into "_crypt_"

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

Some conditional additions to misc.c regarding _libssh2_send() and
_libssh2_recv().

-------------

Regards,
Leif
Attachment (libssh2-1.4.2-morphos.patch): application/octet-stream, 7652 bytes
_______________________________________________
(Continue reading)

Permalink | Reply |
headers
Daniel Stenberg | 2 Aug 2012 00:00
Picon
Favicon
Gravatar

Re: libssh2 MorphOS patch

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:

+#ifdef HAVE_IOCTLSOCKET_CASE
+#include <proto/socket.h>
+#endif
(Continue reading)

Permalink | Reply |
headers
Leif Salomonsson | 2 Aug 2012 11:00

Re: libssh2 MorphOS patch

Hello Daniel,

On 2012-08-01, you wrote:
> 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...

Yes these functions are defines on MorphOS.

> 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.

Ok, noted.

>> 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?

No, just changed ->connect().
(Continue reading)

Permalink | Reply |
headers
Leif Salomonsson | 2 Aug 2012 11:39

Re: libssh2 MorphOS patch

Hello Daniel,

On 2012-08-01, you wrote:

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

Oops, you are right. 

> #ifdef HAVE_PROTO_SOCKET_H
> #include <proto/socket.h>
> #endif

Ok.

> BTW, MorphOS is not present here and I'd appreciate if you'd add it: 
>
https://sourceforge.net/apps/mediawiki/predef/index.php?title=Operating_Systems

Hmm.. I have no access to change that page.

Anyway, morphos uses __MORPHOS__.

Regards,
Leif

_______________________________________________
libssh2-devel http://cool.haxx.se/cgi-bin/mailman/listinfo/libssh2-devel
(Continue reading)

Permalink | Reply |
headers
Leif Salomonsson | 2 Aug 2012 21:17

Re: libssh2 MorphOS patch

Hello 

Ok. version 2 of the patch.
Removed the proto/socket line, wasnt needed.
And fixed some other stuff we talked about.

Only the HAVE_IOCTLSOCKET_CASE thingy left..
Waiting for your reply on my reasoning there. 

Regards,
Leif
Attachment (libssh2-1.4.2-morphos.patch): application/octet-stream, 6495 bytes
_______________________________________________
libssh2-devel http://cool.haxx.se/cgi-bin/mailman/listinfo/libssh2-devel
Permalink | Reply |
headers
Peter Stuge | 4 Aug 2012 14:38
Picon

Re: libssh2 MorphOS patch

Hi,

Leif Salomonsson wrote:
> Only the HAVE_IOCTLSOCKET_CASE thingy left..
> Waiting for your reply on my reasoning there. 

The patch needs more work. Here are some comments.

> diff -u libssh2-1.4.2_orig/src/agent.c libssh2-1.4.2/src/agent.c
> --- libssh2-1.4.2_orig/src/agent.c	Mon Mar  5 20:04:56 2012
> +++ libssh2-1.4.2/src/agent.c	Thu Aug  2 10:32:49 2012
>  <at>  <at>  -122,7 +122,7  <at>  <at> 
>  };
>  
>  struct agent_ops {
> -    agent_connect_func connect;
> +    agent_connect_func _connect_;

Are you saying that on this platform you have a C preprocessor
#defines for connect, send, recv, and crypt, which thus are stolen
from the global namespace? I think that is one insanely broken platform!

>:(

If this is indeed the case, and if we actually do want to support
such stupidity, then please at the very least use *sane* names
instead of adding horrible underscores.

> +++ libssh2-1.4.2/src/transport.c	Tue Jul 31 12:52:44 2012
>  <at>  <at>  -139,7 +139,7  <at>  <at>
(Continue reading)

Permalink | Reply |
headers
Leif Salomonsson | 4 Aug 2012 17:01

Re: libssh2 MorphOS patch

Hello Peter,

On 2012-08-04, you wrote:
> Hi,

> Leif Salomonsson wrote:
>> Only the HAVE_IOCTLSOCKET_CASE thingy left..
>> Waiting for your reply on my reasoning there. 

> The patch needs more work. Here are some comments.

>> diff -u libssh2-1.4.2_orig/src/agent.c libssh2-1.4.2/src/agent.c
>> --- libssh2-1.4.2_orig/src/agent.c   Mon Mar  5 20:04:56 2012
>> +++ libssh2-1.4.2/src/agent.c    Thu Aug  2 10:32:49 2012
>>  <at>  <at>  -122,7 +122,7  <at>  <at> 
>>  };
>>  
>>  struct agent_ops {
>> -    agent_connect_func connect;
>> +    agent_connect_func _connect_;

> Are you saying that on this platform you have a C preprocessor
> #defines for connect, send, recv, and crypt, which thus are stolen
> from the global namespace? I think that is one insanely broken platform!

I agree it is not great having bsdsocket functions as macros,
especially since this API uses such short and generic names.
I will look into an alternative to using the macros, making a 
linklib that encapsulates the macros for example.
But in the mean-time maybe not totally bad idea to rename
(Continue reading)

Permalink | Reply |
headers
Peter Stuge | 4 Aug 2012 17:23
Picon

Re: libssh2 MorphOS patch

Hi!

Leif Salomonsson wrote:
> I will look into an alternative to using the macros, making a 
> linklib that encapsulates the macros for example.

Will that work, given that the macros exist in some header file that
presumably needs to be included?

> > please at the very least use *sane* names
> 
> Sure. How about cb_send() and cb_receive() (cb_= callback). Now
> we have actually improved the names because it becomes clear
> they are not the same as bsdsocket send() and recv() (different
> return values).

Yes, this would be an excellent improvement. Would you please send
that as a separate patch?

> >> -        if (session->remote.crypt->crypt(session, source,
> >> +        if (session->remote.crypt->_crypt_(session, source,
> 
> > Wouldn't you have to change the session->remote.crypt name as well?
> 
> No, GCC is OK with that, likely because "crypt->" is not a function call.

I guess it's #define crypt(foo) then, and CPP will only match the
word including parenthesis.

//Peter
(Continue reading)

Permalink | Reply |
headers
Leif Salomonsson | 4 Aug 2012 19:31

Re: libssh2 MorphOS patch

Hello Peter,

On 2012-08-04, you wrote:
> Hi!

> Leif Salomonsson wrote:
>> I will look into an alternative to using the macros, making a 
>> linklib that encapsulates the macros for example.

> Will that work, given that the macros exist in some header file that
> presumably needs to be included?

I'm sure it would have.. but I just found out one can simply disable the 
inline macros by a using define, so now it compiles just fine without any 
name changes :) New patch attached.

>> > please at the very least use *sane* names
>> 
>> Sure. How about cb_send() and cb_receive() (cb_= callback). Now
>> we have actually improved the names because it becomes clear
>> they are not the same as bsdsocket send() and recv() (different
>> return values).

> Yes, this would be an excellent improvement. Would you please send
> that as a separate patch?

Ok.

>> >> -        if (session->remote.crypt->crypt(session, source,
>> >> +        if (session->remote.crypt->_crypt_(session, source,
(Continue reading)

Permalink | Reply |
headers
Leif Salomonsson | 9 Aug 2012 18:31

Re: send/recv callback names (was: libssh2 MorphOS patch)

Hello Peter,

On 2012-08-04, you wrote:

>> Sure. How about cb_send() and cb_receive() (cb_= callback). Now
>> we have actually improved the names because it becomes clear
>> they are not the same as bsdsocket send() and recv() (different
>> return values).

> Yes, this would be an excellent improvement. Would you please send
> that as a separate patch?

Attached it. 
Now looking at it I'm not sure it does so much good after all but hey, do
as you like :)

It was created after the morphos patch I submitted earlier.

Regards,
Leif
Attachment (libssh2-1.4.2-cb_.patch): application/octet-stream, 2409 bytes
_______________________________________________
libssh2-devel http://cool.haxx.se/cgi-bin/mailman/listinfo/libssh2-devel
Permalink | Reply |

Gmane