On 5/20/11 3:05 PM, Martin Storsjö wrote:
On Fri, 20 May 2011, Luca Barbato wrote:

The connect() timeout can take minutes, gets misreported as EIO and
isn't interruptible.
---
  libavformat/tcp.c |   16 +++++++++++-----
  1 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/libavformat/tcp.c b/libavformat/tcp.c
index 0cb3ae3..cdab57c 100644
--- a/libavformat/tcp.c
+++ b/libavformat/tcp.c
@@ -45,6 +45,7 @@ static int tcp_open(URLContext *h, const char *uri, int flags)
      char buf[256];
      int ret;
      socklen_t optlen;
+    int timeout = 20;
      char hostname[1024],proto[1024],path[1024];
      char portstr[10];


The unit of timeout needs to be specified somewhere. And isn't 20 x 100 ms
a bit low?

I'm not sure which could be a good default and which is not.

@@ -57,6 +58,9 @@ static int tcp_open(URLContext *h, const char *uri, int flags)
      if (p) {
          if (av_find_info_tag(buf, sizeof(buf), "listen", p))
              listen_socket = 1;
+        if (av_find_info_tag(buf, sizeof(buf), "timeout", p)) {
+            timeout = strtol(buf, NULL, 10);
+        }
      }
      memset(&hints, 0, sizeof(hints));
      hints.ai_family = AF_UNSPEC;
@@ -73,6 +77,7 @@ static int tcp_open(URLContext *h, const char *uri, int flags)
      cur_ai = ai;

   restart:
+    ret = AVERROR(EIO);
      fd = socket(cur_ai->ai_family, cur_ai->ai_socktype, cur_ai->ai_protocol);
      if (fd<  0)
          goto fail;
@@ -86,11 +91,10 @@ static int tcp_open(URLContext *h, const char *uri, int 
flags)
          fd = fd1;
      } else {
   redo:
+        ff_socket_nonblock(fd, 1);
          ret = connect(fd, cur_ai->ai_addr, cur_ai->ai_addrlen);
      }

-    ff_socket_nonblock(fd, 1);
-

Doesn't this miss setting listening sockets nonblocking?

You are right.

      if (ret<  0) {
          struct pollfd p = {fd, POLLOUT, 0};
          if (ff_neterrno() == AVERROR(EINTR)) {
@@ -105,7 +109,7 @@ static int tcp_open(URLContext *h, const char *uri, int 
flags)
              goto fail;

          /* wait until we are connected or until abort */
-        for(;;) {
+        while(timeout--) {
              if (url_interrupt_cb()) {
                  ret = AVERROR_EXIT;
                  goto fail1;
@@ -114,7 +118,10 @@ static int tcp_open(URLContext *h, const char *uri, int 
flags)
              if (ret>  0)
                  break;
          }
-
+        if (ret<= 0) {
+            ret = AVERROR(ETIMEDOUT);
+            goto fail;
+        }
          /* test error */
          optlen = sizeof(ret);
          getsockopt (fd, SOL_SOCKET, SO_ERROR,&ret,&optlen);
@@ -144,7 +151,6 @@ static int tcp_open(URLContext *h, const char *uri, int 
flags)
              closesocket(fd);
          goto restart;
      }
-    ret = AVERROR(EIO);

When this is removed, I think you need to set ret to ff_neterrno() in this
error checking branch:

         if (ff_neterrno() != AVERROR(EINPROGRESS)&&
             ff_neterrno() != AVERROR(EAGAIN))
             goto fail;

Before that, ret only is the return value from connect or bind.

You are right as well.

lu
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel

Reply via email to