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) Or something like that. -- Måns Rullgård [email protected] _______________________________________________ libav-devel mailing list [email protected] https://lists.libav.org/mailman/listinfo/libav-devel
