On date Tuesday 2011-04-12 10:23:54 +0200, Anton Khirnov wrote:
> From: Stefano Sabatini <stefano.sabatini-l...@poste.it>
>
> 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 <an...@khirnov.net>
> ---
> 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.
_______________________________________________
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel
