Subject: Re: Patch - keepalive packet

Re: Patch - keepalive packet

From: Daniel Stenberg <daniel_at_haxx.se>
Date: Mon, 9 Dec 2013 11:41:42 +0100 (CET)

On Thu, 28 Nov 2013, Jiří Ševčík wrote:

> we've created another patch that added
> libssh2_check_keepalive(LIBSSH2_SESSION * session) function and adds new
> parameter to LIBSSH_SESSION - keepalive_count for storing count of received
> keepalives.

1. I would expect all keepalive functions to use the prefix libssh2_keepalive

2. You don't provide documentation for the function so it makes it a bit hard
for me to fully grasp how you intend to use this and what its purpose is.

Is getting the keepalive counter (and resetting it) useful in any way to an
application? If so, why is it called 'check' and not 'counter' or something?
Possibly even reset_counter as that's what it does while returning the old
value.

> Problem occurs when session receives keepalive reply. This packet is decoded
> as SSH_MSG_REQUEST_FAILURE but in packet type switch in packet.c file isn't
> handled as this type.

It's been a while since I checked this code, but is this really what's
intended?

It seems like a bit of an overstatement that SSH_MSG_REQUEST_FAILURE is always
because of keepalive, and unconditionally never adding it to the incoming
package queue will hide it from those who check for it. I could only find one
such function right now though: channel_forward_listen().

-- 
  / daniel.haxx.se

_______________________________________________
libssh2-devel http://cool.haxx.se/cgi-bin/mailman/listinfo/libssh2-devel
Received on 2013-12-09