Skip to content

Commit fd5cc7e

Browse files
jlskuzddennedy
jlskuz
and
ddennedy
authored
Add static code analysis workflow with Cppcheck (#1161)
* Add static code analysis workflow with Cppcheck * Fix dereferencing 'pChild1' after it is deleted * Fix realloc failure * Fix cppcheck on macros * Fix VERSION macro for cppcheck * Add some cppcheck suppressions * Fix index array out of bounds * Fix 2 more realloc failures * Fix 2 small memory leaks reported by cppcheck * Fix cppcheck error on invalid arg value -1 is equivalent to string::npos, but that is the parameter default and not needed. * Add some more cppcheck suppressions * Fix another memleak on orig_localenamel * Fix cppcheck fails to parse some files --------- Co-authored-by: Dan Dennedy <dan@dennedy.org>
1 parent 03962c1 commit fd5cc7e

File tree

16 files changed

+143
-35
lines changed

16 files changed

+143
-35
lines changed

‎.github/workflows/static-code-analysis.yml‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
name: static-code-analysis
2+
3+
on: [push, pull_request]
4+
5+
jobs:
6+
cppcheck:
7+
name: Cppcheck
8+
runs-on: ubuntu-latest
9+
env:
10+
extra-args: >-
11+
-i src/modules/decklink/darwin
12+
-i src/modules/decklink/linux
13+
-i src/modules/decklink/win
14+
-i src/modules/glaxnimate/glaxnimate/
15+
-i src/modules/plus/ebur128/
16+
-i src/modules/xml/common.c
17+
-i src/win32/strptime.c
18+
--include=src/framework/mlt_log.h
19+
--include=src/framework/mlt_types.h
20+
--library=cppcheck.cfg
21+
--suppress=ctuOneDefinitionRuleViolation
22+
--suppress=syntaxError:src/modules/xml/common.c
23+
steps:
24+
- uses: actions/checkout@v4
25+
- name: Install Cppcheck
26+
run: sudo apt-get -qq -y install cppcheck
27+
- name: Run Cppcheck
28+
run: cppcheck src/ -j $(nproc) --force --inline-suppr --library=qt --error-exitcode=1 --template="::{severity} file={file},line={line},col={column}::{message}" ${{ env.extra-args }}

‎CMakeLists.txt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ option(GPL3 "Enable GPLv3 components" ON)
1212
option(BUILD_TESTING "Enable tests" OFF)
1313
option(BUILD_DOCS "Enable Doxygen documentation" OFF)
1414
option(CLANG_FORMAT "Enable Clang Format" ON)
15-
option(BUILD_TESTS_WITH_QT6 "Build test against Qt 6" OFF)
15+
option(BUILD_TESTS_WITH_QT6 "Build test against Qt 6" ON)
1616

1717
option(MOD_AVFORMAT "Enable avformat module" ON)
1818
option(MOD_DECKLINK "Enable DeckLink module" ON)

‎cppcheck.cfg‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
<?xml version="1.0"?>
2+
<def format="2">
3+
<define name="VERSION" value="&quot;cppcheck&quot;"/>
4+
<define name="__int64" value="long long"/>
5+
</def>

‎makefile‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,3 +15,19 @@ codespell:
1515
codespell -w -q 3 \
1616
-L shotcut,sav,boundry,percentil,readded,uint,ith,sinc,amin,childs,seeked,writen \
1717
-S ChangeLog,cJSON.c,cJSON.h,RtAudio.cpp,RtAudio.h,*.rej,mlt_wrap.*
18+
19+
cppcheck:
20+
cppcheck src/ --force --quiet --inline-suppr --library=qt --error-exitcode=1 \
21+
-j $(shell nproc) \
22+
-i src/modules/decklink/darwin \
23+
-i src/modules/decklink/linux \
24+
-i src/modules/decklink/win \
25+
-i src/modules/glaxnimate/glaxnimate/ \
26+
-i src/modules/plus/ebur128/ \
27+
-i src/modules/xml/common.c \
28+
-i src/win32/strptime.c \
29+
--include=src/framework/mlt_log.h \
30+
--include=src/framework/mlt_types.h \
31+
--library=cppcheck.cfg \
32+
--suppress=ctuOneDefinitionRuleViolation \
33+
--suppress=syntaxError:src/modules/xml/common.c

‎src/framework/mlt_animation.c‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -742,7 +742,14 @@ char *mlt_animation_serialize_cut_tf(mlt_animation self,
742742
while (used + item_len + 2 > size) // +2 for ';' and NULL
743743
{
744744
size += 1000;
745-
ret = realloc(ret, size);
745+
char *tmp = realloc(ret, size);
746+
if (!tmp) {
747+
free(ret);
748+
mlt_property_close(item.property);
749+
mlt_property_close(time_property);
750+
return NULL;
751+
}
752+
ret = tmp;
746753
}
747754

748755
// Append item delimiter (;) if needed.

‎src/framework/mlt_image.c‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -402,7 +402,7 @@ mlt_colorspace mlt_image_colorspace_id(const char *name)
402402
if (!value && strcmp(name, "0"))
403403
value = -1; // strtol returned an error;
404404

405-
for (int i = 0; i < sizeof(colorspaces); i++) {
405+
for (int i = 0; i < sizeof(colorspaces) / sizeof(colorspaces[0]); i++) {
406406
const char *s = mlt_image_colorspace_name(colorspaces[i]);
407407
if (value == colorspaces[i] || !strcmp(s, name))
408408
return colorspaces[i];
@@ -463,7 +463,7 @@ mlt_color_primaries mlt_image_color_pri_id(const char *name)
463463
if (!value && strcmp(name, "0"))
464464
value = -1; // strtol returned an error;
465465

466-
for (int i = 0; name && i < sizeof(primaries); i++) {
466+
for (int i = 0; name && i < sizeof(primaries) / sizeof(primaries[0]); i++) {
467467
const char *s = mlt_image_color_pri_name(primaries[i]);
468468
if (value == primaries[i] || !strcmp(s, name))
469469
return primaries[i];

‎src/framework/mlt_pool.c‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -182,8 +182,10 @@ static void *pool_fetch(mlt_pool self)
182182
release->references = 1;
183183

184184
// Determine the ptr
185+
// The memory is not leaked - it's returned via ptr
185186
ptr = (char *) release + sizeof(struct mlt_release_s);
186187
}
188+
// cppcheck-suppress memleak
187189
}
188190

189191
// Unlock the pool

‎src/framework/mlt_property.c‎

Lines changed: 54 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
* \brief Property class definition
44
* \see mlt_property_s
55
*
6-
* Copyright (C) 2003-2024 Meltytech, LLC
6+
* Copyright (C) 2003-2025 Meltytech, LLC
77
*
88
* This library is free software; you can redistribute it and/or
99
* modify it under the terms of the GNU Lesser General Public
@@ -37,6 +37,17 @@
3737
#include <stdlib.h>
3838
#include <string.h>
3939

40+
// Platforms with native strtod_l support
41+
#if defined(__GLIBC__) || defined(__APPLE__) || (defined(HAVE_STRTOD_L) && !defined(__OpenBSD__))
42+
#define HAVE_LOCALE_STRTOD_L 1
43+
#endif
44+
45+
// Platforms requiring manual locale handling (excluding Windows)
46+
#if !defined(__GLIBC__) && !defined(__APPLE__) && !defined(_WIN32) && !defined(HAVE_STRTOD_L) \
47+
&& !defined(__OpenBSD__)
48+
#define NEED_LOCALE_SAVE_RESTORE 1
49+
#endif
50+
4051
/** Bit pattern used internally to indicated representations available.
4152
*/
4253

@@ -318,8 +329,7 @@ static int time_clock_to_frames(mlt_property self, const char *s, double fps, ml
318329
s = copy;
319330
pos = strrchr(s, ':');
320331

321-
#if !defined(__GLIBC__) && !defined(__APPLE__) && !defined(_WIN32) && !defined(HAVE_STRTOD_L) \
322-
&& !defined(__OpenBSD__)
332+
#ifdef NEED_LOCALE_SAVE_RESTORE
323333
char *orig_localename = NULL;
324334
if (locale) {
325335
// Protect damaging the global locale from a temporary locale on another thread.
@@ -334,7 +344,7 @@ static int time_clock_to_frames(mlt_property self, const char *s, double fps, ml
334344
#endif
335345

336346
if (pos) {
337-
#if defined(__GLIBC__) || defined(__APPLE__) || defined(HAVE_STRTOD_L) && !defined(__OpenBSD__)
347+
#ifdef HAVE_LOCALE_STRTOD_L
338348
if (locale)
339349
seconds = strtod_l(pos + 1, NULL, locale);
340350
else
@@ -350,16 +360,15 @@ static int time_clock_to_frames(mlt_property self, const char *s, double fps, ml
350360
minutes = atoi(s);
351361
}
352362
} else {
353-
#if defined(__GLIBC__) || defined(__APPLE__) || defined(HAVE_STRTOD_L) && !defined(__OpenBSD__)
363+
#ifdef HAVE_LOCALE_STRTOD_L
354364
if (locale)
355365
seconds = strtod_l(s, NULL, locale);
356366
else
357367
#endif
358368
seconds = strtod(s, NULL);
359369
}
360370

361-
#if !defined(__GLIBC__) && !defined(__APPLE__) && !defined(_WIN32) && !defined(HAVE_STRTOD_L) \
362-
&& !defined(__OpenBSD__)
371+
#ifdef NEED_LOCALE_SAVE_RESTORE
363372
if (locale) {
364373
// Restore the current locale
365374
setlocale(LC_NUMERIC, orig_localename);
@@ -513,22 +522,47 @@ int mlt_property_get_int(mlt_property self, double fps, mlt_locale_t locale)
513522
static double mlt_property_atof(mlt_property self, double fps, mlt_locale_t locale)
514523
{
515524
const char *value = self->prop_string;
525+
#ifdef NEED_LOCALE_SAVE_RESTORE
526+
char *orig_localename = NULL;
527+
#endif
516528

517529
if (fps > 0 && strchr(value, ':')) {
530+
#ifdef NEED_LOCALE_SAVE_RESTORE
531+
if (locale) {
532+
// Protect damaging the global locale from a temporary locale on another thread.
533+
pthread_mutex_lock(&self->mutex);
534+
535+
// Get the current locale
536+
orig_localename = strdup(setlocale(LC_NUMERIC, NULL));
537+
538+
// Set the new locale
539+
setlocale(LC_NUMERIC, locale);
540+
}
541+
#endif
542+
double result;
518543
if (strchr(value, '.') || strchr(value, ','))
519-
return time_clock_to_frames(self, value, fps, locale);
544+
result = time_clock_to_frames(self, value, fps, locale);
520545
else
521-
return time_code_to_frames(self, value, fps);
546+
result = time_code_to_frames(self, value, fps);
547+
548+
#ifdef NEED_LOCALE_SAVE_RESTORE
549+
if (locale) {
550+
// Restore the current locale
551+
setlocale(LC_NUMERIC, orig_localename);
552+
free(orig_localename);
553+
pthread_mutex_unlock(&self->mutex);
554+
}
555+
#endif
556+
return result;
522557
} else {
523558
char *end = NULL;
524559
double result;
525560

526-
#if defined(__GLIBC__) || defined(__APPLE__) || defined(HAVE_STRTOD_L) && !defined(__OpenBSD__)
561+
#ifdef HAVE_LOCALE_STRTOD_L
527562
if (locale)
528563
result = strtod_l(value, &end, locale);
529564
else
530565
#elif !defined(_WIN32)
531-
char *orig_localename = NULL;
532566
if (locale) {
533567
// Protect damaging the global locale from a temporary locale on another thread.
534568
pthread_mutex_lock(&self->mutex);
@@ -545,8 +579,7 @@ static double mlt_property_atof(mlt_property self, double fps, mlt_locale_t loca
545579
if (end && end[0] == '%')
546580
result /= 100.0;
547581

548-
#if !defined(__GLIBC__) && !defined(__APPLE__) && !defined(_WIN32) && !defined(HAVE_STRTOD_L) \
549-
&& !defined(__OpenBSD__)
582+
#ifdef NEED_LOCALE_SAVE_RESTORE
550583
if (locale) {
551584
// Restore the current locale
552585
setlocale(LC_NUMERIC, orig_localename);
@@ -1156,13 +1189,15 @@ int mlt_property_is_numeric(mlt_property self, mlt_locale_t locale)
11561189
// If not already numeric but string is numeric.
11571190
if ((!result && self->types & mlt_prop_string) && self->prop_string) {
11581191
char *p = NULL;
1192+
#ifdef NEED_LOCALE_SAVE_RESTORE
1193+
char *orig_localename = NULL;
1194+
#endif
11591195

1160-
#if defined(__GLIBC__) || defined(__APPLE__) || defined(HAVE_STRTOD_L) && !defined(__OpenBSD__)
1196+
#ifdef HAVE_LOCALE_STRTOD_L
11611197
if (locale)
11621198
strtod_l(self->prop_string, &p, locale);
11631199
else
11641200
#elif !defined(_WIN32)
1165-
char *orig_localename = NULL;
11661201
if (locale) {
11671202
// Protect damaging the global locale from a temporary locale on another thread.
11681203
pthread_mutex_lock(&self->mutex);
@@ -1177,8 +1212,7 @@ int mlt_property_is_numeric(mlt_property self, mlt_locale_t locale)
11771212

11781213
strtod(self->prop_string, &p);
11791214

1180-
#if !defined(__GLIBC__) && !defined(__APPLE__) && !defined(_WIN32) && !defined(HAVE_STRTOD_L) \
1181-
&& !defined(__OpenBSD__)
1215+
#ifdef NEED_LOCALE_SAVE_RESTORE
11821216
if (locale) {
11831217
// Restore the current locale
11841218
setlocale(LC_NUMERIC, orig_localename);
@@ -1893,8 +1927,7 @@ mlt_rect mlt_property_get_rect(mlt_property self, mlt_locale_t locale)
18931927
char *p = NULL;
18941928
int count = 0;
18951929

1896-
#if !defined(__GLIBC__) && !defined(__APPLE__) && !defined(_WIN32) && !defined(HAVE_STRTOD_L) \
1897-
&& !defined(__OpenBSD__)
1930+
#ifdef NEED_LOCALE_SAVE_RESTORE
18981931
char *orig_localename = NULL;
18991932
if (locale) {
19001933
// Protect damaging the global locale from a temporary locale on another thread.
@@ -1910,7 +1943,7 @@ mlt_rect mlt_property_get_rect(mlt_property self, mlt_locale_t locale)
19101943

19111944
while (*value) {
19121945
double temp;
1913-
#if defined(__GLIBC__) || defined(__APPLE__) || defined(HAVE_STRTOD_L) && !defined(__OpenBSD__)
1946+
#ifdef HAVE_LOCALE_STRTOD_L
19141947
if (locale)
19151948
temp = strtod_l(value, &p, locale);
19161949
else
@@ -1951,8 +1984,7 @@ mlt_rect mlt_property_get_rect(mlt_property self, mlt_locale_t locale)
19511984
count++;
19521985
}
19531986

1954-
#if !defined(__GLIBC__) && !defined(__APPLE__) && !defined(_WIN32) && !defined(HAVE_STRTOD_L) \
1955-
&& !defined(__OpenBSD__)
1987+
#ifdef NEED_LOCALE_SAVE_RESTORE
19561988
if (locale) {
19571989
// Restore the current locale
19581990
setlocale(LC_NUMERIC, orig_localename);

‎src/modules/avformat/producer_avformat.c‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1472,6 +1472,7 @@ static int seek_video(producer_avformat self,
14721472
timestamp -= 2 / av_q2d(self->video_time_base);
14731473
if (timestamp < 0)
14741474
timestamp = 0;
1475+
// cppcheck-suppress syntaxError
14751476
mlt_log_debug(MLT_PRODUCER_SERVICE(producer),
14761477
"seeking timestamp %" PRId64 " position " MLT_POSITION_FMT
14771478
" expected " MLT_POSITION_FMT " last_pos %" PRId64 "\n",

‎src/modules/gdk/producer_pango.c‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -298,9 +298,14 @@ mlt_producer producer_pango_init(const char *filename)
298298
while (fgets(line, 80, f)) {
299299
size += strlen(line) + 1;
300300
if (markup) {
301-
markup = realloc(markup, size);
302-
if (markup)
301+
char *new_markup = realloc(markup, size);
302+
if (new_markup) {
303+
markup = new_markup;
303304
strcat(markup, line);
305+
} else {
306+
// Allocation failed: keep existing content and stop appending
307+
break;
308+
}
304309
} else {
305310
markup = strdup(line);
306311
}

0 commit comments

Comments
 (0)