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

[Carl Eugen Hoyos]

2018-12-10 23:53 GMT+01:00, Mark Thompson <sw at jkqxz.net>:
> On 10/12/2018 00:57, Carl Eugen Hoyos wrote:
>> 2018-12-09 19:03 GMT+01:00, Mark Thompson <sw at 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 at 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 at 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