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

Reply via email to