I'll introduce a new internal function set named
_libssh2_store_u32
_libssh2_store_u64
_libssh2_store_str
That can be used all through the library to build binary outgoing
packets. Using these instead of the current approach removes
hundreds of lines from the library while at the same time greatly
enhances readability. I've not yet fully converted everything to
use these functions.
I've converted LOTS of 'unsigned long' to 'size_t' where
data/string lengths are dealt with internally. This is The Right
Thing and it will help us make the transition to our
size_t-polished API later on as well.
I'm removing the PACKET_* error codes. They were originally
introduced as a set of separate error codes from the transport
layer, but having its own set of errors turned out to be very
awkward and they were then converted into a set of #defines that
simply maps them to the global libssh2 error codes instead. Now,
I'l take the next logical step and simply replace the PACKET_*
defines with the actual LIBSSH2_ERROR_* defines. It will increase
readability and decrease confusion.
I also separated packet stuff into its own packet.h header file.
We reserve ^libssh2_ for public symbols and we use _libssh2 as
prefix for internal ones. I fixed the intendation of all these
edits with emacs afterwards, which then changed it slightly more
than just _libssh2_error() expressions but I didn't see any
obvious problems.
When we ignore the EAGAIN from the transport layer within channel_write, we
now drain the outgoing transport layer buffer so that remainders in that
won't cause any problems in the next invoke of _libssh2_transport_write()
When sending data in a loop, we must not return EAGAIN if we
managed to send data in an earlier round. This was reported in
bug #126 => http://libssh2.stuge.se/ticket/126
As reported in bug report #166http://libssh2.stuge.se/ticket/166
by 'ptjm', the maximum window size must be less crazy for libssh2
to do better with more server implementations. I did not do any
testing to see how this changes raw SCP performance, but the
maximum window size is still almost 4MB. This also has the upside
that libssh2 will use less memory.
libssh2_error() no longer allocates a string and only accepts a const
error string. I also made a lot of functions use the construct of
return libssh2_error(...) instead of having one call to
libssh2_error() and then a separate return call. In several of those
cases I then also changed the former -1 return code to a more
detailed one - something that I think will not change behaviors
anywhere but it's worth keeping an eye open for any such.
Sending SSH_MSG_CHANNEL_CLOSE without channel EOF is explicitly allowed
in RFC 4254, but some non-conforming servers will hang or time out when
the channel is closed before EOF.
Other common clients send and receive EOF before closing, there are no
drawbacks, and some servers need it to work correctly.
The forward accepting was not done right before, and the
packet_queue_listener function didn't assign a necessary
variable. All fixed by Juzna. I (Daniel) modified the
forward_accept() change somewhat.
If the channel is already at EOF or even closed at the end of the
libssh2_channel_read_ex() function and there's no data to return,
we need to signal that back. We may have gotten that info while
draining the incoming transport layer until EAGAIN so we must not
be fooled by that return code.
libssh2_channel_wait_closed() had a bad loop waiting for the
channel to close, as it could easily miss the info and then if
the socket would be silent from that moment the funtion would
hang if in blocking-mode or just return EAGAIN wrongly to the
app. The drain-transport loop now correctly checks if the close
has arrived.
In theory we could split larger buffers into several smaller
packets to pass to transport_write(), but for now we instead only
deal with the first 32K in this call and assume the app will call
this function again with the rest! The 32K size is a
conservative limit based on the text in RFC4253 section 6.1.
internal code was changed to use that instead of wrongly using
libssh2_channel_read_ex(). Some files now need to include
channel.h to get this proto.
channel_read() calls libssh2_error() properly on transport_read()
failures
channel_read() was adjusted to not "invent" EAGAIN return code in
case the transport_read() didn't return it
channel_close() now returns 0 or error code, as
documented. Previously it would return number of bytes read in
the last read, which was confusing (and useless).
I made this change just to easier grep for "return .*EAGAIN" cases
as they should be very rare or done wrongly. Already worked to find
a flaw, marked with "TODO FIXME THIS IS WRONG" in channel.c. I also
fixed a few cases to become more general returns now when we have
more unified return codes internally.
it is important that only the transport layer can generate EAGAIN
error codes so that we limit where we need to set direction bits
and more. When the local window is too small to send data we simply
stop trying to send and (risk) returning zero in
_libssh2_channel_write()
On many places in the code there have been laziness return -1
statements lying around that should be fixed to return sensible
error codes. Here's a take at fixing a few offenders.
transport layer better, to avoid draining the window size when sending large
packets. I also fixed the return code for it to return the number of bytes
handled in this single invoke (and not the cumulative amount).
in loops until it returns < 0 when we call it, as if we just call it once we
may drain the socket for data and then leave unused in-memory data that we
won't detect because the socket is back to idle...
until _after_ we're done as otherwise we might not get called again properly
to clean the entire thing since the close state is checked in
libssh2_channel_free
and introduced a transport.h header.
* Fixed the blocking mode to only change behavior not the actual underlying
socket mode so we now always work with non-blocking sockets. This also
introduces a new rule of thumb in libssh2 code: we don't call the
external function calls internally. We use the internal (non-blocking)
ones!
* libssh2_channel_receive_window_adjust2 was added and
libssh2_channel_receive_window_adjust is now deprecated
* Introduced "local" header files with prototypes etc for different parts
instead of cramming everything into libssh2_priv.h. channel.h is the
first.
frequent, use a few less struct fields in the channel struct and improved
reading from the network with libssh2_packet_read(). I also modified the
windowing algorithm and now use a much larger window. This greatly enhances
SSH/SCP performance. I also increased the size of the buffer the transport
layer uses from 4k to 16K.
some tests:
* cut off "_ex" from several internal function names
* corrected some log outputs
* simplified libssh2_channel_read_ex() and made it much faster in the process
* cut out {{{ and }}} comments that were incorrect anyway
* fixed sftp_packet_ask() to return the correct packet by using memcmp() and
not strncmp()
* fixed mkdir()'s wait for packet to use the correct request_id - it
semi-worked previously because strncmp() in sftp_packet_ask() made it
match far too easily.
* took away the polling functionality from sftp_packet_ask() since it wasn't
used
libssh2_channel_x11_req_ex() function that made it produce a crappy random
chunk of data. Peter Stuge improved the fix to not do out-of-boundary
writes. I (Daniel Stenberg) replaced the snprintf() with a plain sprintf()
since the size argument wasn't adding anything anyway.
