[FFmpeg-devel] [PATCH] avformat/avio: Remove no-op code in url_find_protocol().

[Wan-Teh Chang]

Hi Muhammad,

On Thu, Jul 6, 2017 at 7:56 PM, Muhammad Faiz <mfcc64 at gmail.com> wrote:
> On Fri, Jul 7, 2017 at 9:18 AM, Wan-Teh Chang
> <wtc-at-google.com at ffmpeg.org> wrote:
>> In url_find_protocol(), proto_str is either "file" or a string
>> consisting of only the characters in URL_SCHEME_CHARS, which does not
>> include ','. Therefore the strchr(proto_str, ',') call always returns
>> NULL.
>>
>> Note: The code was added in commit
>> 6161c41817f6e53abb3021d67ca0f19def682718.
>>
>> Signed-off-by: Wan-Teh Chang <wtc at google.com>
>> ---
>>  libavformat/avio.c | 2 --
>>  1 file changed, 2 deletions(-)
>>
>> diff --git a/libavformat/avio.c b/libavformat/avio.c
>> index 1e79c9dd5c..64248e098b 100644
>> --- a/libavformat/avio.c
>> +++ b/libavformat/avio.c
>> @@ -263,8 +263,6 @@ static const struct URLProtocol *url_find_protocol(const char *filename)
>>          av_strlcpy(proto_str, filename,
>>                     FFMIN(proto_len + 1, sizeof(proto_str)));
>>
>> -    if ((ptr = strchr(proto_str, ',')))
>> -        *ptr = '\0';
>
> What about handling "subfile," ?

I don't know what url_find_protocol() is intended to do, but I can
show you what it actually does.

Here is the relevant code in libavformat/avio.c:

======================================================================
#define URL_SCHEME_CHARS                        \
    "abcdefghijklmnopqrstuvwxyz"                \
    "ABCDEFGHIJKLMNOPQRSTUVWXYZ"                \
    "0123456789+-."

static const struct URLProtocol *url_find_protocol(const char *filename)
{
    const URLProtocol **protocols;
    char proto_str[128], proto_nested[128], *ptr;
    size_t proto_len = strspn(filename, URL_SCHEME_CHARS);
    int i;

    if (filename[proto_len] != ':' &&
        (strncmp(filename, "subfile,", 8) || !strchr(filename +
proto_len + 1, ':')) ||
        is_dos_path(filename))
        strcpy(proto_str, "file");
    else
        av_strlcpy(proto_str, filename,
                   FFMIN(proto_len + 1, sizeof(proto_str)));

    if ((ptr = strchr(proto_str, ',')))
        *ptr = '\0';
======================================================================

Since I don't know how "subfile," should be handled by
url_find_protocol(), I ran the following three test inputs in the
debugger:

If |filename| is "subfile,", then proto_len is 7 and the if branch
copies "file" into proto_str.

If |filename| is "subfile,abcdefg", then proto_len is 7 and the if
branch copies "file" into proto_str.

If |filename| is "subfile,abcdefg:hijk", then proto_len is 7 and the
else branch copies "subfile" into proto_str.

Is this the expected behavior?

Note: The code snippet shows proto_str cannot possibly contain ','.
This is why the strchr(proto_str, ',') call always returns NULL.

Thanks,
Wan-Teh Chang