Discussion:
[FFmpeg-devel] [PATCH]lavf/matroskadec: Do not use strncat() to limit copying a one-char constant
Carl Eugen Hoyos
2018-12-06 22:26:55 UTC
Permalink
Hi!

Attached patch silences a new gcc warning, alternative would be to
disable the warning.

Please comment, Carl Eugen
Carl Eugen Hoyos
2018-12-10 00:57:34 UTC
Permalink
Post by Carl Eugen Hoyos
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
Date: Thu, 6 Dec 2018 23:23:12 +0100
Subject: [PATCH] lavf/matroskadec: Do not use strncat() to limit copying a
one-char constant.
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
Mark Thompson
2018-12-10 22:53:33 UTC
Permalink
Post by Carl Eugen Hoyos
Post by Carl Eugen Hoyos
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
Date: Thu, 6 Dec 2018 23:23:12 +0100
Subject: [PATCH] lavf/matroskadec: Do not use strncat() to limit copying a
one-char constant.
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
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.
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
2018-12-10 23:43:39 UTC
Permalink
Post by Mark Thompson
Post by Carl Eugen Hoyos
Post by Carl Eugen Hoyos
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
Date: Thu, 6 Dec 2018 23:23:12 +0100
Subject: [PATCH] lavf/matroskadec: Do not use strncat() to limit copying a
one-char constant.
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
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.
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

Loading...