Matt Lilley wrote:
> Ok, that seems reasonable to me. I've attached a new patch that does this
> (I think), and still passes my test cases.
I'll let you and Daniel sort the implementation details. Just one
comment:
> +++ b/src/session.h
> @@ -51,17 +51,20 @@
> function.
>
> */
> -#define BLOCK_ADJUST(rc,sess,x) \
> +#define BLOCK_ADJUST(rc,sess,x) { \
> + time_t entry_time = time (NULL); \
> do { \
> rc = x; \
> /* the order of the check below is important to properly deal with the
> case when the 'sess' is freed */ \
> if((rc != LIBSSH2_ERROR_EAGAIN) || !sess->api_block_mode) \
> break; \
> - rc = _libssh2_wait_socket(sess); \
> + rc = _libssh2_wait_socket(sess, entry_time); \
> if(rc) \
> break; \
> - } while(1)
> + } while(1); \
> +}
> +
No. Using do {} while() must not change (you add braces) since it
allows the macro to successfully be used as a function in all
circumstances in C code. In this case you would add an outer
do {} while() as such:
#define BLOCK_ADJUST(rc,sess,x) { \
do { \
time_t entry_time ... \
do { \
...
} while(1);
} while(0)
Also, please make sure to maintain code style (whitespace changes) in
the code that you work on. Thanks!
Btw, it doesn't make sense to have while(1) immediately preceded by a
conditional break IMO. Might as well fix that too while touching
this:
.. while(!rc);
> @@ -69,7 +72,8 @@
> * immediately. If the API is blocking and we get a NULL we check the errno
> * and *only* if that is EAGAIN we loop and wait for socket action.
> */
> -#define BLOCK_ADJUST_ERRNO(ptr,sess,x) \
> +#define BLOCK_ADJUST_ERRNO(ptr,sess,x) { \
Same comment as above.
//Peter
_______________________________________________
libssh2-devel http://cool.haxx.se/cgi-bin/mailman/listinfo/libssh2-devel
Received on 2011-05-03