Re: [FFmpeg-devel] [PATCH] avformat/avio: Remove no-op code in url_find_protocol().
On Fri, Jul 7, 2017 at 1:32 PM, Muhammad Faiz wrote:
> On Fri, Jul 7, 2017 at 11:27 AM, Wan-Teh Chang
> wrote:
>> Hi Muhammad,
>>
>> On Thu, Jul 6, 2017 at 7:56 PM, Muhammad Faiz wrote:
>>> On Fri, Jul 7, 2017 at 9:18 AM, Wan-Teh Chang
>>> 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
---
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.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avformat/avio: Remove no-op code in url_find_protocol().
On Fri, Jul 7, 2017 at 11:27 AM, Wan-Teh Chang
wrote:
> Hi Muhammad,
>
> On Thu, Jul 6, 2017 at 7:56 PM, Muhammad Faiz wrote:
>> On Fri, Jul 7, 2017 at 9:18 AM, Wan-Teh Chang
>> 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
>>> ---
>>> 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.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avformat/avio: Remove no-op code in url_find_protocol().
Hi Muhammad,
On Thu, Jul 6, 2017 at 7:56 PM, Muhammad Faiz wrote:
> On Fri, Jul 7, 2017 at 9:18 AM, Wan-Teh Chang
> 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
>> ---
>> 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
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avformat/avio: Remove no-op code in url_find_protocol().
On Fri, Jul 7, 2017 at 9:18 AM, Wan-Teh Chang 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 > --- > 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," ? ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] avformat/avio: Remove no-op code in url_find_protocol().
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 --- 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'; av_strlcpy(proto_nested, proto_str, sizeof(proto_nested)); if ((ptr = strchr(proto_nested, '+'))) *ptr = '\0'; -- 2.13.2.725.g09c95d1e9-goog ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
