Subject: Re: libssh2 MorphOS patch

Re: libssh2 MorphOS patch

From: Leif Salomonsson <dev_at_blubbedev.net>
Date: 04 Aug 2012 16:01:04 +0100

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
>> @@ -122,7 +122,7 @@
>> };
>>
>> 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
the functions anyway, see below:

>>:(

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

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

>> +++ libssh2-1.4.2/src/transport.c Tue Jul 31 12:52:44 2012
>> @@ -139,7 +139,7 @@
>> assert((len % blocksize) == 0);
>>
>> while (len >= blocksize) {
>> - 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.

>> @@ -167,7 +167,7 @@
>> unsigned char macbuf[MAX_MACSIZE];
>> struct transportpacket *p = &session->packet;
>> int rc;
>> -
>> +

> Please never make whitespace changes at the same time as other
> changes.

Right.

Regards,
Leif

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