diff mbox

[FFmpeg-devel] lavf/matroskadec: Do not use strncat() to limit copying a one-char constant

Message ID CAB0OVGqJqxhwVkQV4pVyGAWp3mce7ttY9KcgUx5WP2dvfPEQTA@mail.gmail.com
State Accepted
Headers show

Commit Message

Carl Eugen Hoyos Dec. 10, 2018, 12:57 a.m. UTC 
2018-12-09 19:03 GMT+01:00, Mark Thompson <sw@jkqxz.net>:
> On 06/12/2018 22:26, Carl Eugen Hoyos wrote:
>> Hi!
>>
>> Attached patch silences a new gcc warning, alternative would be to
>> disable the warning.
>>
>> Please comment, Carl Eugen
>>
>>
>> From dd49cddc6fad136222d4a168301059d55fea4a4c Mon Sep 17 00:00:00 2001
>> From: Carl Eugen Hoyos <ceffmpeg@gmail.com>
>> Date: Thu, 6 Dec 2018 23:23:12 +0100
>> Subject: [PATCH] lavf/matroskadec: Do not use strncat() to limit copying a
>>  one-char constant.
>>
>> Silences a warning:
>> libavformat/matroskadec.c: In function 'webm_dash_manifest_cues':
>> libavformat/matroskadec.c:3947:13: warning: 'strncat' specified bound 1
>> equals source length [-Wstringop-overflow=]
>>              strncat(buf, ",", 1);
>>              ^~~~~~~~~~~~~~~~~~~~
>> ---
>>  libavformat/matroskadec.c |    2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
>> index 2daa1db..df820b4 100644
>> --- a/libavformat/matroskadec.c
>> +++ b/libavformat/matroskadec.c
>> @@ -3944,7 +3944,7 @@ static int webm_dash_manifest_cues(AVFormatContext
>> *s, int64_t init_range)
>>          }
>>          end += ret;
>>          if (i != s->streams[0]->nb_index_entries - 1) {
>> -            strncat(buf, ",", 1);
>> +            strcat(buf, ",");
>>              end++;
>>          }
>>      }
>> --
>> 1.7.10.4
>>
>
> LGTM.
>
> (Optional: perhaps nicer to remove that code fragment with the str(n?)cat
> completely by including the comma in the snprintf above, as '"%s", i !=
> s->streams[0]->nb_index_entries - 1 ? "," : ""'?)

New patch attached.

Please review, Carl Eugen

Comments

Mark Thompson Dec. 10, 2018, 10:53 p.m. UTC | #1 
On 10/12/2018 00:57, Carl Eugen Hoyos wrote:
> 2018-12-09 19:03 GMT+01:00, Mark Thompson <sw@jkqxz.net>:
>> On 06/12/2018 22:26, Carl Eugen Hoyos wrote:
>>> Hi!
>>>
>>> Attached patch silences a new gcc warning, alternative would be to
>>> disable the warning.
>>>
>>> Please comment, Carl Eugen
>>>
>>>
>>> From dd49cddc6fad136222d4a168301059d55fea4a4c Mon Sep 17 00:00:00 2001
>>> From: Carl Eugen Hoyos <ceffmpeg@gmail.com>
>>> Date: Thu, 6 Dec 2018 23:23:12 +0100
>>> Subject: [PATCH] lavf/matroskadec: Do not use strncat() to limit copying a
>>>  one-char constant.
>>>
>>> Silences a warning:
>>> libavformat/matroskadec.c: In function 'webm_dash_manifest_cues':
>>> libavformat/matroskadec.c:3947:13: warning: 'strncat' specified bound 1
>>> equals source length [-Wstringop-overflow=]
>>>              strncat(buf, ",", 1);
>>>              ^~~~~~~~~~~~~~~~~~~~
>>> ---
>>>  libavformat/matroskadec.c |    2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
>>> index 2daa1db..df820b4 100644
>>> --- a/libavformat/matroskadec.c
>>> +++ b/libavformat/matroskadec.c
>>> @@ -3944,7 +3944,7 @@ static int webm_dash_manifest_cues(AVFormatContext
>>> *s, int64_t init_range)
>>>          }
>>>          end += ret;
>>>          if (i != s->streams[0]->nb_index_entries - 1) {
>>> -            strncat(buf, ",", 1);
>>> +            strcat(buf, ",");
>>>              end++;
>>>          }
>>>      }
>>> --
>>> 1.7.10.4
>>>
>>
>> LGTM.
>>
>> (Optional: perhaps nicer to remove that code fragment with the str(n?)cat
>> completely by including the comma in the snprintf above, as '"%s", i !=
>> s->streams[0]->nb_index_entries - 1 ? "," : ""'?)
> 
> New patch attached.
> 
> Please review, Carl Eugen
> 
> 
> From 082bce9706a4c326187ae543d8b8aa93424c48b0 Mon Sep 17 00:00:00 2001
> From: Carl Eugen Hoyos <ceffmpeg@gmail.com>
> Date: Mon, 10 Dec 2018 01:55:15 +0100
> Subject: [PATCH] lavf/matroskadec: Do not use strncat() to limit copying a
>  one-char constant.
> 
> Instead add the character to the snprintf above as suggested by Mark.
> 
> Silences a warning:
> libavformat/matroskadec.c: In function 'webm_dash_manifest_cues':
> libavformat/matroskadec.c:3947:13: warning: 'strncat' specified bound 1 equals source length [-Wstringop-overflow=]
>              strncat(buf, ",", 1);
>              ^~~~~~~~~~~~~~~~~~~~
> ---
>  libavformat/matroskadec.c |    7 ++-----
>  1 file changed, 2 insertions(+), 5 deletions(-)
> 
> diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
> index 2daa1db..4ad99db 100644
> --- a/libavformat/matroskadec.c
> +++ b/libavformat/matroskadec.c
> @@ -3936,17 +3936,14 @@ static int webm_dash_manifest_cues(AVFormatContext *s, int64_t init_range)
>      strcpy(buf, "");
>      for (i = 0; i < s->streams[0]->nb_index_entries; i++) {
>          int ret = snprintf(buf + end, 20,
> -                           "%" PRId64, s->streams[0]->index_entries[i].timestamp);
> +                           "%" PRId64"%s", s->streams[0]->index_entries[i].timestamp,
> +                           i != s->streams[0]->nb_index_entries - 1 ? "," : "");
>          if (ret <= 0 || (ret == 20 && i ==  s->streams[0]->nb_index_entries - 1)) {
>              av_log(s, AV_LOG_ERROR, "timestamp too long.\n");
>              av_free(buf);
>              return AVERROR_INVALIDDATA;
>          }
>          end += ret;
> -        if (i != s->streams[0]->nb_index_entries - 1) {
> -            strncat(buf, ",", 1);
> -            end++;
> -        }
>      }
>      av_dict_set(&s->streams[0]->metadata, CUE_TIMESTAMPS, buf, 0);
>      av_free(buf);
> -- 
> 1.7.10.4

Yep, LGTM.

Thanks,

- Mark
Carl Eugen Hoyos Dec. 10, 2018, 11:43 p.m. UTC | #2 
2018-12-10 23:53 GMT+01:00, Mark Thompson <sw@jkqxz.net>:
> On 10/12/2018 00:57, Carl Eugen Hoyos wrote:
>> 2018-12-09 19:03 GMT+01:00, Mark Thompson <sw@jkqxz.net>:
>>> On 06/12/2018 22:26, Carl Eugen Hoyos wrote:
>>>> Hi!
>>>>
>>>> Attached patch silences a new gcc warning, alternative would be to
>>>> disable the warning.
>>>>
>>>> Please comment, Carl Eugen
>>>>
>>>>
>>>> From dd49cddc6fad136222d4a168301059d55fea4a4c Mon Sep 17 00:00:00 2001
>>>> From: Carl Eugen Hoyos <ceffmpeg@gmail.com>
>>>> Date: Thu, 6 Dec 2018 23:23:12 +0100
>>>> Subject: [PATCH] lavf/matroskadec: Do not use strncat() to limit copying
>>>> a
>>>>  one-char constant.
>>>>
>>>> Silences a warning:
>>>> libavformat/matroskadec.c: In function 'webm_dash_manifest_cues':
>>>> libavformat/matroskadec.c:3947:13: warning: 'strncat' specified bound 1
>>>> equals source length [-Wstringop-overflow=]
>>>>              strncat(buf, ",", 1);
>>>>              ^~~~~~~~~~~~~~~~~~~~
>>>> ---
>>>>  libavformat/matroskadec.c |    2 +-
>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
>>>> index 2daa1db..df820b4 100644
>>>> --- a/libavformat/matroskadec.c
>>>> +++ b/libavformat/matroskadec.c
>>>> @@ -3944,7 +3944,7 @@ static int webm_dash_manifest_cues(AVFormatContext
>>>> *s, int64_t init_range)
>>>>          }
>>>>          end += ret;
>>>>          if (i != s->streams[0]->nb_index_entries - 1) {
>>>> -            strncat(buf, ",", 1);
>>>> +            strcat(buf, ",");
>>>>              end++;
>>>>          }
>>>>      }
>>>> --
>>>> 1.7.10.4
>>>>
>>>
>>> LGTM.
>>>
>>> (Optional: perhaps nicer to remove that code fragment with the str(n?)cat
>>> completely by including the comma in the snprintf above, as '"%s", i !=
>>> s->streams[0]->nb_index_entries - 1 ? "," : ""'?)
>>
>> New patch attached.
>>
>> Please review, Carl Eugen
>>
>>
>> From 082bce9706a4c326187ae543d8b8aa93424c48b0 Mon Sep 17 00:00:00 2001
>> From: Carl Eugen Hoyos <ceffmpeg@gmail.com>
>> Date: Mon, 10 Dec 2018 01:55:15 +0100
>> Subject: [PATCH] lavf/matroskadec: Do not use strncat() to limit copying a
>>  one-char constant.
>>
>> Instead add the character to the snprintf above as suggested by Mark.
>>
>> Silences a warning:
>> libavformat/matroskadec.c: In function 'webm_dash_manifest_cues':
>> libavformat/matroskadec.c:3947:13: warning: 'strncat' specified bound 1
>> equals source length [-Wstringop-overflow=]
>>              strncat(buf, ",", 1);
>>              ^~~~~~~~~~~~~~~~~~~~
>> ---
>>  libavformat/matroskadec.c |    7 ++-----
>>  1 file changed, 2 insertions(+), 5 deletions(-)
>>
>> diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
>> index 2daa1db..4ad99db 100644
>> --- a/libavformat/matroskadec.c
>> +++ b/libavformat/matroskadec.c
>> @@ -3936,17 +3936,14 @@ static int webm_dash_manifest_cues(AVFormatContext
>> *s, int64_t init_range)
>>      strcpy(buf, "");
>>      for (i = 0; i < s->streams[0]->nb_index_entries; i++) {
>>          int ret = snprintf(buf + end, 20,
>> -                           "%" PRId64,
>> s->streams[0]->index_entries[i].timestamp);
>> +                           "%" PRId64"%s",
>> s->streams[0]->index_entries[i].timestamp,
>> +                           i != s->streams[0]->nb_index_entries - 1 ? ","
>> : "");
>>          if (ret <= 0 || (ret == 20 && i ==
>> s->streams[0]->nb_index_entries - 1)) {
>>              av_log(s, AV_LOG_ERROR, "timestamp too long.\n");
>>              av_free(buf);
>>              return AVERROR_INVALIDDATA;
>>          }
>>          end += ret;
>> -        if (i != s->streams[0]->nb_index_entries - 1) {
>> -            strncat(buf, ",", 1);
>> -            end++;
>> -        }
>>      }
>>      av_dict_set(&s->streams[0]->metadata, CUE_TIMESTAMPS, buf, 0);
>>      av_free(buf);
>> --
>> 1.7.10.4
>
> Yep, LGTM.

Patch applied, Carl Eugen
diff mbox

Patch

From 082bce9706a4c326187ae543d8b8aa93424c48b0 Mon Sep 17 00:00:00 2001
From: Carl Eugen Hoyos <ceffmpeg@gmail.com>
Date: Mon, 10 Dec 2018 01:55:15 +0100
Subject: [PATCH] lavf/matroskadec: Do not use strncat() to limit copying a
 one-char constant.

Instead add the character to the snprintf above as suggested by Mark.

Silences a warning:
libavformat/matroskadec.c: In function 'webm_dash_manifest_cues':
libavformat/matroskadec.c:3947:13: warning: 'strncat' specified bound 1 equals source length [-Wstringop-overflow=]
             strncat(buf, ",", 1);
             ^~~~~~~~~~~~~~~~~~~~
---
 libavformat/matroskadec.c |    7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
index 2daa1db..4ad99db 100644
--- a/libavformat/matroskadec.c
+++ b/libavformat/matroskadec.c
@@ -3936,17 +3936,14 @@  static int webm_dash_manifest_cues(AVFormatContext *s, int64_t init_range)
     strcpy(buf, "");
     for (i = 0; i < s->streams[0]->nb_index_entries; i++) {
         int ret = snprintf(buf + end, 20,
-                           "%" PRId64, s->streams[0]->index_entries[i].timestamp);
+                           "%" PRId64"%s", s->streams[0]->index_entries[i].timestamp,
+                           i != s->streams[0]->nb_index_entries - 1 ? "," : "");
         if (ret <= 0 || (ret == 20 && i ==  s->streams[0]->nb_index_entries - 1)) {
             av_log(s, AV_LOG_ERROR, "timestamp too long.\n");
             av_free(buf);
             return AVERROR_INVALIDDATA;
         }
         end += ret;
-        if (i != s->streams[0]->nb_index_entries - 1) {
-            strncat(buf, ",", 1);
-            end++;
-        }
     }
     av_dict_set(&s->streams[0]->metadata, CUE_TIMESTAMPS, buf, 0);
     av_free(buf);
-- 
1.7.10.4