[FFmpeg-devel] [PATCH] avio: Check for FF_NETERROR(EAGAIN) in retry_transfer_wrapper

[Martin Storsjö]

On Mon, 21 Feb 2011, Ronald S. Bultje wrote:

> On Sat, Feb 19, 2011 at 1:17 PM, Martin Storsj? <martin at martin.st> wrote:
> > On Thu, 17 Feb 2011, Ronald S. Bultje wrote:
> >
> >> Hi,
> >>
> >> On Thu, Feb 17, 2011 at 11:22 AM, Martin Storsj? <martin at martin.st> wrote:
> >> > On Thu, 17 Feb 2011, Nicolas George wrote:
> >> >
> >> >> Le nonidi 29 pluvi?se, an CCXIX, Luca Barbato a ?crit?:
> >> >> > It would mean repeat that for every network protocol, might worth a try
> >> >> > though.
> >> >>
> >> >> All (almost?) protocols use the ff_neterrno() macro, which is defined, on
> >> >> windows, as:
> >> >>
> >> >> #define ff_neterrno() (-WSAGetLastError())
> >> >>
> >> >> It could become:
> >> >>
> >> >> static inline int ff_neterrno(void)
> >> >> {
> >> >> ? ? int err = -WSAGetLastError();
> >> >> ? ? return err == -WSAEWOULDBLOCK ? AVERROR(EAGAIN) : err;
> >> >> }
> >> >>
> >> >> This would become a good place for similar mappings should they become
> >> >> necessary in the future.
> >> >
> >> > If this change is done, all uses of FF_NETERROR(EAGAIN) need to be
> >> > switched to AVERROR(EAGAIN) at the same time.
> >>
> >> Sounds good to me, I've always disliked this FF_NETERROR() thing, it's
> >> just very prone to stupid bugs that we can't really find because
> >> they're win32-only and error-only.
> >
> > Indeed, getting rid of this would be great. However, that requires us to
> > add mappings in ff_neterrno() for all the network errors that we want to
> > catch via AVERROR(), that could lead to similar sublte bugs.
> >
> > What's worse, though, is that we use EPROTONOSUPPORT, ETIMEDOUT and
> > ECONNREFUSED, too, and these ones don't exist in windows errno.h, only in
> > the WSA-prefixed versions, so there's no header-provided value that we can
> > map them to.
> 
> Can we map them to self-defined error codes on Windows? Made-up
> values, is what I mean. I know that won't act well with strerror(),
> but there isn't really an ideal solution for this.

Hmm, yes. The question is how to handle it most neatly. Should we add e.g. 
something like this in libavutil/error.h:

#ifndef ECONNREFUSED
#define ECONNREFUSED 31337
#endif

Or alternatively, something like this:

#ifdef ECONNREFUSED
#define AVERROR_ECONNREFUSED -ECONNREFUSED
#else
#define AVERROR_ECONNREFUSED -31337
#endif

This variant is cleaner, but requires all the code checking for these 
errors to be changed to use the underscore variant of the error code.

> > What do you think about this, is it worth fixing it this way, while still
> > leaving a few cases of FF_NETERROR() scattered around for the errors that
> > don't have a non-WSA-prefixed version in the windows errno.h?
> 
> Ideally we'd kill FF_NETERROR() completely.

Indeed, that would be great.

// Martin