Martin Storsjö <[email protected]> writes: > On Tue, 19 Jun 2012, Måns Rullgård wrote: > >> Martin Storsjö <[email protected]> writes: >> >>> On Tue, 19 Jun 2012, Måns Rullgård wrote: >>> >>>> Martin Storsjö <[email protected]> writes: >>>> >>>>> diff --git a/libavformat/tcp.c b/libavformat/tcp.c >>>>> index 7e348f7..e7c6210 100644 >>>>> --- a/libavformat/tcp.c >>>>> +++ b/libavformat/tcp.c >>>>> @@ -141,10 +141,12 @@ static int tcp_open(URLContext *h, const char *uri, >>>>> int flags) >>>>> optlen = sizeof(ret); >>>>> getsockopt (fd, SOL_SOCKET, SO_ERROR, &ret, &optlen); >>>>> if (ret != 0) { >>>>> + char errbuf[100]; >>>>> + ret = AVERROR(ret); >>>>> + av_strerror(ret, errbuf, sizeof(errbuf)); >>>>> av_log(h, AV_LOG_ERROR, >>>>> "TCP connection to %s:%d failed: %s\n", >>>>> - hostname, port, strerror(ret)); >>>>> - ret = AVERROR(ret); >>>>> + hostname, port, errbuf); >>>>> goto fail; >>>>> } >>>>> } >>>> >>>> This is all wrong. getsockopt() indicates error by returning -1. In >>>> case of error, the value stored in 'ret', if any, appears to be >>>> undefined (the spec does not mention anything). >>> >>> That's unrelated to this patch, but yes. >>> >>> I guess something like this would be better? >>> >>> if (getsockopt(fd, SOL_SOCKET, SO_ERROR, &ret, &optlen)) >>> ret = AVUNERROR(ff_neterrno()); // To get the error back into the >>> normal errno range >> >> Reading the code more carefully, it's more complicated than that. >> getsockopt(SO_ERROR) is used to check for asynchronous errors on the >> socket, so the original code makes some sense assuming the values >> retrieved with SO_ERROR are usual errno values (I can't find any >> information on this). That said, the return value of getsockopt() >> should also be checked. >> >> So this could work: >> >> if (getsockopt(fd, SOL_SOCKET, SO_ERROR, &ret, &optlen) && (ret = errno) || >> ret) > > Yes, although you'd need ret = AVUNERROR(ff_neterrno()) instead, for > windows compat. And that basicaly boiled down to what I suggested > above (where I intended to keep that above the normal code, so what I > meant was something like this in total: > > if (getsockopt(fd, SOL_SOCKET, SO_ERROR, &ret, &optlen)) > ret = AVUNERROR(ff_neterrno()); > if (ret != 0) { // the current code > .... > }
OK, that should work. -- Måns Rullgård [email protected] _______________________________________________ libav-devel mailing list [email protected] https://lists.libav.org/mailman/listinfo/libav-devel
