From 1863d8377a4ae2ec3a06a8ab816e8d15348c27b1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A9ter=20Bohner=20=28xzvf=29?= Date: Thu, 7 Jul 2022 14:35:59 +0200 Subject: [PATCH 1/4] Fix memory leak in ffParseProfFileValues, if more than 4 queries are passed (unsetValues is on the heap) and the file is not found. --- src/common/properties.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/common/properties.c b/src/common/properties.c index 15763d34d..de39be817 100644 --- a/src/common/properties.c +++ b/src/common/properties.c @@ -109,7 +109,11 @@ bool ffParsePropFileValues(const char* filename, uint32_t numQueries, FFpropquer FILE* file = fopen(filename, "r"); if(file == NULL) + { + if(unsetValues != valueStorage) + free(unsetValues); return false; + } if(allSet) { From 7bc85b239746b64c94eebad8e3917da328d71702 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A9ter=20Bohner=20=28xzvf=29?= Date: Thu, 7 Jul 2022 15:19:56 +0200 Subject: [PATCH 2/4] Coalesce cleanup in ffParsePropFileValues --- src/common/properties.c | 24 +++++++++--------------- 1 file changed, 9 insertions(+), 15 deletions(-) diff --git a/src/common/properties.c b/src/common/properties.c index de39be817..df9a7b60c 100644 --- a/src/common/properties.c +++ b/src/common/properties.c @@ -92,6 +92,8 @@ bool ffParsePropLines(const char* lines, const char* start, FFstrbuf* buffer) bool ffParsePropFileValues(const char* filename, uint32_t numQueries, FFpropquery* queries) { + bool returnValue = true; + bool valueStorage[4]; bool* unsetValues; @@ -108,19 +110,10 @@ bool ffParsePropFileValues(const char* filename, uint32_t numQueries, FFpropquer } FILE* file = fopen(filename, "r"); - if(file == NULL) + if(file == NULL || allSet) { - if(unsetValues != valueStorage) - free(unsetValues); - return false; - } - - if(allSet) - { - fclose(file); - if(unsetValues != valueStorage) - free(unsetValues); - return true; + returnValue = allSet; + goto cleanup; } char* line = NULL; @@ -143,12 +136,13 @@ bool ffParsePropFileValues(const char* filename, uint32_t numQueries, FFpropquer if(line != NULL) free(line); - fclose(file); - +cleanup: + if(file != NULL) + fclose(file); if(unsetValues != valueStorage) free(unsetValues); - return true; + return returnValue; } bool ffParsePropFile(const char* filename, const char* start, FFstrbuf* buffer) From fa1382604373ba44c7c4fb8ad4c8fcbd753dbe78 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A9ter=20Bohner=20=28xzvf=29?= Date: Fri, 8 Jul 2022 09:05:00 +0200 Subject: [PATCH 3/4] Revert "Coalesce cleanup in ffParsePropFileValues" This reverts commit 7bc85b239746b64c94eebad8e3917da328d71702. --- src/common/properties.c | 24 +++++++++++++++--------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/src/common/properties.c b/src/common/properties.c index df9a7b60c..de39be817 100644 --- a/src/common/properties.c +++ b/src/common/properties.c @@ -92,8 +92,6 @@ bool ffParsePropLines(const char* lines, const char* start, FFstrbuf* buffer) bool ffParsePropFileValues(const char* filename, uint32_t numQueries, FFpropquery* queries) { - bool returnValue = true; - bool valueStorage[4]; bool* unsetValues; @@ -110,10 +108,19 @@ bool ffParsePropFileValues(const char* filename, uint32_t numQueries, FFpropquer } FILE* file = fopen(filename, "r"); - if(file == NULL || allSet) + if(file == NULL) { - returnValue = allSet; - goto cleanup; + if(unsetValues != valueStorage) + free(unsetValues); + return false; + } + + if(allSet) + { + fclose(file); + if(unsetValues != valueStorage) + free(unsetValues); + return true; } char* line = NULL; @@ -136,13 +143,12 @@ bool ffParsePropFileValues(const char* filename, uint32_t numQueries, FFpropquer if(line != NULL) free(line); -cleanup: - if(file != NULL) - fclose(file); + fclose(file); + if(unsetValues != valueStorage) free(unsetValues); - return returnValue; + return true; } bool ffParsePropFile(const char* filename, const char* start, FFstrbuf* buffer) From 292a809703bbf331d02bef7716340a413b7bc0b0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A9ter=20Bohner=20=28xzvf=29?= Date: Fri, 8 Jul 2022 09:09:59 +0200 Subject: [PATCH 4/4] ffParsePropValues: Hoisted fopen call to simplify error handling --- src/common/properties.c | 23 ++++++----------------- 1 file changed, 6 insertions(+), 17 deletions(-) diff --git a/src/common/properties.c b/src/common/properties.c index de39be817..3c45854f6 100644 --- a/src/common/properties.c +++ b/src/common/properties.c @@ -88,10 +88,13 @@ bool ffParsePropLines(const char* lines, const char* start, FFstrbuf* buffer) // The following functions return true if the file was found, independently if start was found // Buffers which already contain content are not overwritten // The last occurence of start in the first file will be the one used -// The *Values methods always return true, if all properties were already found before, without testing if the file exists bool ffParsePropFileValues(const char* filename, uint32_t numQueries, FFpropquery* queries) { + FILE* file = fopen(filename, "r"); + if(file == NULL) + return false; + bool valueStorage[4]; bool* unsetValues; @@ -107,21 +110,8 @@ bool ffParsePropFileValues(const char* filename, uint32_t numQueries, FFpropquer allSet = false; } - FILE* file = fopen(filename, "r"); - if(file == NULL) - { - if(unsetValues != valueStorage) - free(unsetValues); - return false; - } - if(allSet) - { - fclose(file); - if(unsetValues != valueStorage) - free(unsetValues); - return true; - } + goto done; char* line = NULL; size_t len = 0; @@ -143,11 +133,10 @@ bool ffParsePropFileValues(const char* filename, uint32_t numQueries, FFpropquer if(line != NULL) free(line); +done: fclose(file); - if(unsetValues != valueStorage) free(unsetValues); - return true; }