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

Muhammad Faiz mfcc64 at gmail.com
Mon Jul 10 03:25:51 EEST 2017


On Fri, Jul 7, 2017 at 1:32 PM, Muhammad Faiz <mfcc64 at gmail.com> wrote:
> On Fri, Jul 7, 2017 at 11:27 AM, Wan-Teh Chang
> <wtc-at-google.com at ffmpeg.org> wrote:
>> 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.
>
> Oh, I see. I was wrong.
>
>>
>> Is this the expected behavior?
>
> I don't know. However it is the previous behavior, so LGTM.
>
> Thank's.

Applied.

Thank's.


More information about the ffmpeg-devel mailing list