From 524096c139405c84ff1f0ddd42d0fef1e7d76545 Mon Sep 17 00:00:00 2001 From: Sebastian Pipping Date: Fri, 21 Feb 2025 22:19:26 +0100 Subject: [PATCH 1/5] tests/benchmark: Rename misleading variable "fd" to "file" --- expat/tests/benchmark/benchmark.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/expat/tests/benchmark/benchmark.c b/expat/tests/benchmark/benchmark.c index 355d83f8..49a2045c 100644 --- a/expat/tests/benchmark/benchmark.c +++ b/expat/tests/benchmark/benchmark.c @@ -62,7 +62,7 @@ int main(int argc, char *argv[]) { XML_Parser parser; char *XMLBuf, *XMLBufEnd, *XMLBufPtr; - FILE *fd; + FILE *file; struct stat fileAttr; int nrOfLoops, bufferSize, i, isFinal; size_t fileSize; @@ -88,8 +88,8 @@ main(int argc, char *argv[]) { return 2; } - fd = fopen(argv[j + 1], "r"); - if (! fd) { + file = fopen(argv[j + 1], "r"); + if (! file) { fprintf(stderr, "could not open file '%s'\n", argv[j + 1]); exit(2); } @@ -102,8 +102,8 @@ main(int argc, char *argv[]) { } XMLBuf = malloc(fileAttr.st_size); - fileSize = fread(XMLBuf, sizeof(char), fileAttr.st_size, fd); - fclose(fd); + fileSize = fread(XMLBuf, sizeof(char), fileAttr.st_size, file); + fclose(file); if (ns) parser = XML_ParserCreateNS(NULL, '!'); From 4de3d65003b3a6d799d561d59d668eac17783e24 Mon Sep 17 00:00:00 2001 From: Sebastian Pipping Date: Fri, 21 Feb 2025 22:24:16 +0100 Subject: [PATCH 2/5] tests/benchmark: Resolve needless use of exit for clarity --- expat/tests/benchmark/benchmark.c | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/expat/tests/benchmark/benchmark.c b/expat/tests/benchmark/benchmark.c index 49a2045c..0f9b1360 100644 --- a/expat/tests/benchmark/benchmark.c +++ b/expat/tests/benchmark/benchmark.c @@ -35,7 +35,6 @@ #include #include #include // ptrdiff_t -#include #include #include #include "expat.h" @@ -52,10 +51,10 @@ # define XML_FMT_STR "s" #endif -static void +static int usage(const char *prog, int rc) { fprintf(stderr, "usage: %s [-n] filename bufferSize nr_of_loops\n", prog); - exit(rc); + return rc; } int @@ -76,12 +75,12 @@ main(int argc, char *argv[]) { ns = 1; j = 1; } else - usage(argv[0], 1); + return usage(argv[0], 1); } } if (argc != j + 4) - usage(argv[0], 1); + return usage(argv[0], 1); if (stat(argv[j + 1], &fileAttr) != 0) { fprintf(stderr, "could not access file '%s'\n", argv[j + 1]); @@ -91,14 +90,14 @@ main(int argc, char *argv[]) { file = fopen(argv[j + 1], "r"); if (! file) { fprintf(stderr, "could not open file '%s'\n", argv[j + 1]); - exit(2); + return 2; } bufferSize = atoi(argv[j + 2]); nrOfLoops = atoi(argv[j + 3]); if (bufferSize <= 0 || nrOfLoops <= 0) { fprintf(stderr, "buffer size and nr of loops must be greater than zero.\n"); - exit(3); + return 3; } XMLBuf = malloc(fileAttr.st_size); @@ -132,7 +131,7 @@ main(int argc, char *argv[]) { XML_GetCurrentColumnNumber(parser)); free(XMLBuf); XML_ParserFree(parser); - exit(4); + return 4; } XMLBufPtr += bufferSize; } while (! isFinal); From 5f4144a6bcb2ef19882d6914fa903597560dc00f Mon Sep 17 00:00:00 2001 From: Sebastian Pipping Date: Fri, 21 Feb 2025 23:25:36 +0100 Subject: [PATCH 3/5] tests/benchmark: Add missing call to fclose --- expat/tests/benchmark/benchmark.c | 1 + 1 file changed, 1 insertion(+) diff --git a/expat/tests/benchmark/benchmark.c b/expat/tests/benchmark/benchmark.c index 0f9b1360..70e39378 100644 --- a/expat/tests/benchmark/benchmark.c +++ b/expat/tests/benchmark/benchmark.c @@ -96,6 +96,7 @@ main(int argc, char *argv[]) { bufferSize = atoi(argv[j + 2]); nrOfLoops = atoi(argv[j + 3]); if (bufferSize <= 0 || nrOfLoops <= 0) { + fclose(file); fprintf(stderr, "buffer size and nr of loops must be greater than zero.\n"); return 3; } From ead919d692f6f0e5069c75397b43d25bbe0bbf17 Mon Sep 17 00:00:00 2001 From: Sebastian Pipping Date: Fri, 21 Feb 2025 22:30:51 +0100 Subject: [PATCH 4/5] tests/benchmark: Resolve (harmless) TOCTTOU .. that was reported by Coverity Scan. https://en.wikipedia.org/wiki/Time-of-check_to_time-of-use --- expat/tests/benchmark/benchmark.c | 24 ++++++++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/expat/tests/benchmark/benchmark.c b/expat/tests/benchmark/benchmark.c index 70e39378..43ef9594 100644 --- a/expat/tests/benchmark/benchmark.c +++ b/expat/tests/benchmark/benchmark.c @@ -32,6 +32,15 @@ USE OR OTHER DEALINGS IN THE SOFTWARE. */ +#define _POSIX_C_SOURCE 1 // fdopen + +#if defined(_MSC_VER) +# include // _open, _close +#else +# include // close +#endif + +#include // open #include #include #include // ptrdiff_t @@ -61,6 +70,7 @@ int main(int argc, char *argv[]) { XML_Parser parser; char *XMLBuf, *XMLBufEnd, *XMLBufPtr; + int fd; FILE *file; struct stat fileAttr; int nrOfLoops, bufferSize, i, isFinal; @@ -82,13 +92,21 @@ main(int argc, char *argv[]) { if (argc != j + 4) return usage(argv[0], 1); - if (stat(argv[j + 1], &fileAttr) != 0) { + fd = open(argv[j + 1], O_RDONLY); + if (fd == -1) { + fprintf(stderr, "could not open file '%s'\n", argv[j + 1]); + return 2; + } + + if (fstat(fd, &fileAttr) != 0) { + close(fd); fprintf(stderr, "could not access file '%s'\n", argv[j + 1]); return 2; } - file = fopen(argv[j + 1], "r"); + file = fdopen(fd, "r"); if (! file) { + close(fd); fprintf(stderr, "could not open file '%s'\n", argv[j + 1]); return 2; } @@ -97,6 +115,7 @@ main(int argc, char *argv[]) { nrOfLoops = atoi(argv[j + 3]); if (bufferSize <= 0 || nrOfLoops <= 0) { fclose(file); + close(fd); fprintf(stderr, "buffer size and nr of loops must be greater than zero.\n"); return 3; } @@ -104,6 +123,7 @@ main(int argc, char *argv[]) { XMLBuf = malloc(fileAttr.st_size); fileSize = fread(XMLBuf, sizeof(char), fileAttr.st_size, file); fclose(file); + close(fd); if (ns) parser = XML_ParserCreateNS(NULL, '!'); From 7f5903483eb2ef2d0cd46cc3d8697f1c7e472b61 Mon Sep 17 00:00:00 2001 From: Sebastian Pipping Date: Fri, 21 Feb 2025 22:35:19 +0100 Subject: [PATCH 5/5] tests/benchmark: Make error messages more technical --- expat/tests/benchmark/benchmark.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/expat/tests/benchmark/benchmark.c b/expat/tests/benchmark/benchmark.c index 43ef9594..a3e3741a 100644 --- a/expat/tests/benchmark/benchmark.c +++ b/expat/tests/benchmark/benchmark.c @@ -100,14 +100,14 @@ main(int argc, char *argv[]) { if (fstat(fd, &fileAttr) != 0) { close(fd); - fprintf(stderr, "could not access file '%s'\n", argv[j + 1]); + fprintf(stderr, "could not fstat file '%s'\n", argv[j + 1]); return 2; } file = fdopen(fd, "r"); if (! file) { close(fd); - fprintf(stderr, "could not open file '%s'\n", argv[j + 1]); + fprintf(stderr, "could not fdopen file '%s'\n", argv[j + 1]); return 2; }