diff mbox

[FFmpeg-devel] avformat/hlsenc: initialize saveptrs

Message ID 20180321193350.2468-1-timo@rothenpieler.org
State Accepted
Commit 3914c8e0e6bac4ce3b3573c6fd3098e2aa4c9ff5
Headers show

Commit Message

Timo Rothenpieler March 21, 2018, 7:33 p.m. UTC
av_strtok calls strspn on a non-NULL *saveptr, so not NULL initializing it is an issue.

Fixes CID #1428568
---
 libavformat/hlsenc.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Timo Rothenpieler March 21, 2018, 7:37 p.m. UTC | #1
Am 21.03.2018 um 20:33 schrieb Timo Rothenpieler:
> av_strtok calls strspn on a non-NULL *saveptr, so not NULL initializing it is an issue.
> 
> Fixes CID #1428568
> ---
>   libavformat/hlsenc.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c
> index b7c6fbde6a..fa17776efe 100644
> --- a/libavformat/hlsenc.c
> +++ b/libavformat/hlsenc.c
> @@ -1873,7 +1873,8 @@ static int parse_cc_stream_mapstring(AVFormatContext *s)
>   {
>       HLSContext *hls = s->priv_data;
>       int nb_ccstreams;
> -    char *p, *q, *saveptr1, *saveptr2, *ccstr, *keyval;
> +    char *p, *q, *ccstr, *keyval;
> +    char *saveptr1 = NULL, *saveptr2 = NULL;
>       const char *val;
>       ClosedCaptionsStream *ccs;

Just realized, the more correct approach is probably to check the 
av_strdup below this for ENOMEM. Not sure about the exact semantics 
there, initializing these still seems like a good safety measure.
Timo Rothenpieler March 25, 2018, 8:13 a.m. UTC | #2
Am 21.03.2018 um 20:37 schrieb Timo Rothenpieler:
> Am 21.03.2018 um 20:33 schrieb Timo Rothenpieler:
>> av_strtok calls strspn on a non-NULL *saveptr, so not NULL 
>> initializing it is an issue.
>>
>> Fixes CID #1428568
>> ---
>>   libavformat/hlsenc.c | 3 ++-
>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c
>> index b7c6fbde6a..fa17776efe 100644
>> --- a/libavformat/hlsenc.c
>> +++ b/libavformat/hlsenc.c
>> @@ -1873,7 +1873,8 @@ static int 
>> parse_cc_stream_mapstring(AVFormatContext *s)
>>   {
>>       HLSContext *hls = s->priv_data;
>>       int nb_ccstreams;
>> -    char *p, *q, *saveptr1, *saveptr2, *ccstr, *keyval;
>> +    char *p, *q, *ccstr, *keyval;
>> +    char *saveptr1 = NULL, *saveptr2 = NULL;
>>       const char *val;
>>       ClosedCaptionsStream *ccs;
> 
> Just realized, the more correct approach is probably to check the 
> av_strdup below this for ENOMEM. Not sure about the exact semantics 
> there, initializing these still seems like a good safety measure.
> 

ping
Jeyapal, Karthick March 26, 2018, 3:49 a.m. UTC | #3
On 3/25/18 1:43 PM, Timo Rothenpieler wrote:
> Am 21.03.2018 um 20:37 schrieb Timo Rothenpieler:

>> Am 21.03.2018 um 20:33 schrieb Timo Rothenpieler:

>>> av_strtok calls strspn on a non-NULL *saveptr, so not NULL 

>>> initializing it is an issue.

>>>

>>> Fixes CID #1428568

>>> ---

>>>   libavformat/hlsenc.c | 3 ++-

>>>   1 file changed, 2 insertions(+), 1 deletion(-)

>>>

>>> diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c

>>> index b7c6fbde6a..fa17776efe 100644

>>> --- a/libavformat/hlsenc.c

>>> +++ b/libavformat/hlsenc.c

>>> @@ -1873,7 +1873,8 @@ static int 

>>> parse_cc_stream_mapstring(AVFormatContext *s)

>>>   {

>>>       HLSContext *hls = s->priv_data;

>>>       int nb_ccstreams;

>>> -    char *p, *q, *saveptr1, *saveptr2, *ccstr, *keyval;

>>> +    char *p, *q, *ccstr, *keyval;

>>> +    char *saveptr1 = NULL, *saveptr2 = NULL;

>>>       const char *val;

>>>       ClosedCaptionsStream *ccs;

>>

>> Just realized, the more correct approach is probably to check the 

>> av_strdup below this for ENOMEM. Not sure about the exact semantics 

>> there, initializing these still seems like a good safety measure.

>>

I agree with you that checking the return value of av_strdup for NULL is a better approach.
But I am fine with this initialization fix as well.
>

> ping

>
Liu Steven March 26, 2018, 12:34 p.m. UTC | #4
> 在 2018年3月26日,上午11:49,Jeyapal, Karthick <kjeyapal@akamai.com> 写道:
> 
> 
> 
> On 3/25/18 1:43 PM, Timo Rothenpieler wrote:
>> Am 21.03.2018 um 20:37 schrieb Timo Rothenpieler:
>>> Am 21.03.2018 um 20:33 schrieb Timo Rothenpieler:
>>>> av_strtok calls strspn on a non-NULL *saveptr, so not NULL 
>>>> initializing it is an issue.
>>>> 
>>>> Fixes CID #1428568
>>>> ---
>>>>  libavformat/hlsenc.c | 3 ++-
>>>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>>> 
>>>> diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c
>>>> index b7c6fbde6a..fa17776efe 100644
>>>> --- a/libavformat/hlsenc.c
>>>> +++ b/libavformat/hlsenc.c
>>>> @@ -1873,7 +1873,8 @@ static int 
>>>> parse_cc_stream_mapstring(AVFormatContext *s)
>>>>  {
>>>>      HLSContext *hls = s->priv_data;
>>>>      int nb_ccstreams;
>>>> -    char *p, *q, *saveptr1, *saveptr2, *ccstr, *keyval;
>>>> +    char *p, *q, *ccstr, *keyval;
>>>> +    char *saveptr1 = NULL, *saveptr2 = NULL;
>>>>      const char *val;
>>>>      ClosedCaptionsStream *ccs;
>>> 
>>> Just realized, the more correct approach is probably to check the 
>>> av_strdup below this for ENOMEM. Not sure about the exact semantics 
>>> there, initializing these still seems like a good safety measure.
>>> 
> I agree with you that checking the return value of av_strdup for NULL is a better approach.
> But I am fine with this initialization fix as well.

Pushed


Thanks
Steven
diff mbox

Patch

diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c
index b7c6fbde6a..fa17776efe 100644
--- a/libavformat/hlsenc.c
+++ b/libavformat/hlsenc.c
@@ -1873,7 +1873,8 @@  static int parse_cc_stream_mapstring(AVFormatContext *s)
 {
     HLSContext *hls = s->priv_data;
     int nb_ccstreams;
-    char *p, *q, *saveptr1, *saveptr2, *ccstr, *keyval;
+    char *p, *q, *ccstr, *keyval;
+    char *saveptr1 = NULL, *saveptr2 = NULL;
     const char *val;
     ClosedCaptionsStream *ccs;