On Tue, Apr 12, 2011 at 07:34:59PM +0200, Stefano Sabatini wrote: > On date Tuesday 2011-04-12 10:23:54 +0200, Anton Khirnov wrote: > > From: Stefano Sabatini <[email protected]> > > > > The new function is more flexible than url_exist(), as it allows to > > specify which access flags to check, and does not require an explicit > > open of the checked resource. > > > > Signed-off-by: Anton Khirnov <[email protected]> > > --- > > libavformat/avio.c | 19 +++++++++++++++++++ > > libavformat/avio.h | 15 +++++++++++++++ > > libavformat/url.h | 1 + > > 3 files changed, 35 insertions(+), 0 deletions(-) > > > > diff --git a/libavformat/avio.c b/libavformat/avio.c > > index ad1f1b4..18b2ee6 100644 > > --- a/libavformat/avio.c > > +++ b/libavformat/avio.c > > @@ -374,6 +374,25 @@ int url_exist(const char *filename) > > return 1; > > } > > > > +int avio_check(const char *url, int flags) > > +{ > > + URLContext *h; > > + int ret = ffurl_alloc(&h, url, flags); > > + if (ret) > > + return ret; > > + > > + if (h->prot->url_check) { > > + ret = h->prot->url_check(h, flags); > > + } else { > > + ret = ffurl_connect(h); > > + if (ret >= 0) > > + ret = flags; > > + } > > + > > + ffurl_close(h); > > + return ret; > > +} > > + > > int64_t ffurl_size(URLContext *h) > > { > > int64_t pos, size; > > diff --git a/libavformat/avio.h b/libavformat/avio.h > > index b980d49..285c9c7 100644 > > --- a/libavformat/avio.h > > +++ b/libavformat/avio.h > > @@ -134,6 +134,7 @@ typedef struct URLProtocol { > > int priv_data_size; > > const AVClass *priv_data_class; > > int flags; > > + int (*url_check)(URLContext *h, int mask); > > } URLProtocol; > > > > typedef struct URLPollEntry { > > @@ -347,6 +348,20 @@ attribute_deprecated int url_close_buf(AVIOContext *s); > > int url_exist(const char *url); > > > > /** > > + * Return AVIO_* access flags corresponding to the access permissions > > + * of the resource in url, or a negative value corresponding to an > > + * AVERROR code in case of failure. The returned access flags are > > + * masked by the value in flags. > > + * > > + * @note This function is intrinsically unsafe, in the sense that the > > + * checked resource may change its existence or permission status from > > + * one call to another. Thus you should not trust the returned value, > > + * unless you are sure that no other processes are accessing the > > + * checked resource. > > + */ > > +int avio_check(const char *url, int flags); > > Please don't apply this since there is a problem with it. > > #define AVIO_RDONLY 0 /**< read-only */ > #define AVIO_WRONLY 1 /**< write-only */ > #define AVIO_RDWR 2 /**< read-write */ > > Indeed AVIO_* are not flags but modes, and it doesn't much sense > OR-ing them, so for example > AVIO_RDONLY|AVIO_WRONLY == AVIO_WRONLY. > > and with this patch we have: > avio_check(url, 0) == 0 -> check for existence > equivalent to > avio_check(url, AVIO_RDONLY) == AVIO_RDONLY -> check for exclusive readability > > which doesn't look right (while the fact that we don't open the > resource still avoids the lock, and thus fixes roundup issue 1663). > > A possible solution discussed with Anton would be to make AVIO_* > constansts proper flags. > > For example we could replace the AVIO_RDONLY and AVIO_WRONLY constants > like this: > > #define AVIO_FLAG_RD 1 > #define AVIO_FLAG_WR 2 > > or simply redefine them at the next major bump, but before that the > logic of this avio_check() is broken. > > Another possibility: > make avio_check() accept and return flags obtained by shifting 1 by > AVIO_* mode value, for example: > > // check for existence > avio_check(url, 0) == 0 > > // check for exclusive readability > avio_check(url, (1<<AVIO_RDONLY)) > 0 > > // check for readability > avio_check(url, (1<<AVIO_RDONLY)|(1<<AVIO_RDWR)) > 0 > > but not very nice from the usability POV. > > I'm greatly sorry for not having noticed this problem before.
I'm in favor of redefining AVIO_* flags during the bump. Unles somebody objects, I'll postpone the patch replacing url_exist with avio_check until the bump and push the rest now, so avio changes can be finally declared finished. During the bump, i'll redefine AVIO_* flags and replace url_exist with avio_check. -- Anton Khirnov
signature.asc
Description: Digital signature
_______________________________________________ libav-devel mailing list [email protected] https://lists.libav.org/mailman/listinfo/libav-devel
