Re: Support external recv and send callbacks

[Joseph Southwell <jsouthwell@xxxxxxxxxxxxx>]

I shouldn’t have said it only works with blocking IO. It is more accurate to say I only know that it works with blocking IO. I haven’t tested it with non blocking IO.
> On May 30, 2017, at 4:31 PM, Joseph Southwell <jsouthwell@xxxxxxxxxxxxx> wrote:
> 
> This only works with blocking IO. (Sort of). I am actually using it with non blocking io, but as far as libssh is concerned it is blocking. I am using c++ implemented stackful coroutines to essentially suspend the stack in my io calls until the non blocking io completes. Proof of concept attached.
> I could put a check in to throw an error if a user tries to set nonblocking io and io callbacks at the same time if you like. 
> I isolated it from ssh_callbacks in an attempt to not step on your feet. Compartmentalized. I can change that if you prefer. 
> 
> <sftp_proof.zip>
> 
>> On May 30, 2017, at 4:04 PM, Aris Adamantiadis <aris@xxxxxxxxxxxx> wrote:
>> 
>> Hi Joseph,
>> 
>> Thanks for your Patch. I have no idea about the call convention to use.
>> That there are two incompatible competing conventions is imho a good
>> sign that we should let a middleware do this for us. Or maybe there's a
>> way to explicitely tell mono or .NET which call convention is being used?
>> 
>> Back to the patch: good work. Is there a reason those callbacks couldn't
>> be merged into struct ssh_callbacks_struct?
>> This would make less callback structures (we already got too many) and
>> these are already backward-compatible proof (size field +
>> ssh_callbacks_init + ssh_callback_exists).
>> Another question, what's happening with poll ? There must be a way for
>> libssh to be awakened when there's data to be read on the socket and
>> when write will not block (flow control). How do you integrate this code
>> into your own main loop at the moment ?
>> 
>> Aris
>> 
>> On 30/05/17 21:30, Joseph Southwell wrote:
>>> You realize if you are going to modify the cmake stuff to
>>> conditionally insert a #define into my callback typedefs there is no
>>> reason you can’t do it to the others if you just disable it by default. 
>>> Here is my patch. I am going to submit it on the bug tracker but if
>>> you want to use it before they apply it be my guest. 
>>> 
>>> 
>>> 
>>> 
>>> 
>>>> On May 30, 2017, at 3:25 PM, Jason Curl
>>>> <jason@xxxxxxxxxxxxxxxxxxxxxxxx
>>>> <mailto:jason@xxxxxxxxxxxxxxxxxxxxxxxx>> wrote:
>>>> 
>>>> Seeing that cmake is being used, its not too hard. When I compile on
>>>> mingw64, the WIN32 micro is already set. Not om Cygwin, bit that's
>>>> not a typical use case. By not too hard I mean about 1 hoir
>>>> implementation, mostly reading. I can support if you want
>>>> 
>>>> 
>>>> 
>>>> Sent from my Windows 10 phone
>>>> 
>>>> 
>>>> 
>>>> *From: *Joseph Southwell <mailto:jsouthwell@xxxxxxxxxxxxx>
>>>> *Sent: *Tuesday 30 May 2017 21:16
>>>> *To: *libssh@xxxxxxxxxx <mailto:libssh@xxxxxxxxxx>
>>>> *Subject: *Re: Support external recv and send callbacks
>>>> 
>>>> 
>>>> 
>>>> So, according to
>>>> http://mono-list.ximian.narkive.com/lfOw5Ldl/mono-net-delegate-incompatibility
>>>> MONO and .NET have different requirements on windows (one requires
>>>> cdecl callbacks and the other requires stdcall). If that is still the
>>>> case there isn’t really right answer without adding build options and
>>>> that gets a little out of the area I am being paid to put effort into. 
>>>> 
>>>>> On May 28, 2017, at 3:19 PM, Jason Curl
>>>> <jason@xxxxxxxxxxxxxxxxxxxxxxxx
>>>> <mailto:jason@xxxxxxxxxxxxxxxxxxxxxxxx>> wrote:
>>>>> 
>>>>> 
>>>>> On 25/05/2017 16:25, Joseph Southwell wrote:
>>>>>> I found a flaw in this and will be resubmitting it.
>>>>> If you're going to make callbacks public, any way to prefix them
>>>> with __stdcall? That would allow .NET to use the callbacks also [1],
>>>> [2], with the only solution I've seen in [3] that is not practical.
>>>>> 
>>>>> It would mean having to add something like WINAPI to the header
>>>> file for the typedefs and if we're compiling on Windows then set it
>>>> to __stdcall (note for 64-bit this is ignored anyway and everything
>>>> is __fastcall).
>>>>> 
>>>>> e.g.
>>>>> #if defined(WIN32)
>>>>> #define WINAPI __stdcall
>>>>> #else
>>>>> #define WINAPI
>>>>> #endif
>>>>> 
>>>>> Note, you can't do that for the existing APIs, because that would
>>>> cause breakage.
>>>>> 
>>>>> I haven't tested on Linux with Mono/dotnet, but I assume here one
>>>> doesnt need to worry.
>>>>>> 
>>>>>>> On May 25, 2017, at 9:44 AM, Joseph Southwell
>>>> <jsouthwell@xxxxxxxxxxxxx <mailto:jsouthwell@xxxxxxxxxxxxx>> wrote:
>>>>>>> 
>>>>>>> This patch allows you to replace the calls to send and recv with
>>>> alternate callbacks. I am using this because I am running an event
>>>> driven network stack with stackfull coroutines in c++. This makes the
>>>> whole thing appear blocking to libssh while still sharing threads
>>>> efficiently. Socket closure is non blocking so I did not add a
>>>> callback for that but if somebody wanted to extend this to work with
>>>> something that isn’t really sockets they would need to add a close
>>>> callback as well since ssh_disconnect calls close on the socket fd.
>>>>>>> <0001-got-external-io-working-in-libssh-client.patch>
>>>>>> 
>>>>> [1] https://msdn.microsoft.com/en-us/library/aa288468(v=vs.71).aspx
>>>> <https://msdn.microsoft.com/en-us/library/aa288468%28v=vs.71%29.aspx>
>>>>> [2] https://msdn.microsoft.com/en-us/library/ektebyzx.aspx
>>>>> 
>>>> [3] https://www.codeproject.com/Articles/12512/Using-the-CDECL-calling-convention-in-C-changing
>>>>> 
>>> 
>> 
>> 
>> 
>