From 402080049bee258d741fc5d5a97683942800e822 Mon Sep 17 00:00:00 2001 From: "Darryl L. Miles" Date: Thu, 13 Feb 2025 08:22:47 +0000 Subject: [PATCH] DBio.c: CodeQL File{MayNot,Never}BeClosed.ql file-handle resource leaks Guided by CodeQL static code analyser. FileMayNotBeClosed.ql FileMayNeverBeClosed.ql Technically the FILE_LOCKS feature leaks the file handle, but maybe this isn't in a perfectly controlled way (with assurance that at some correct point in the program future, all the fd's are eventually closed) --- database/DBio.c | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/database/DBio.c b/database/DBio.c index c4120276..4715b4c6 100644 --- a/database/DBio.c +++ b/database/DBio.c @@ -3875,7 +3875,6 @@ DBCellWrite(cellDef, fileName) const char *cp1; char *cp2, *dotptr; char expandbuf[NAME_SIZE]; - FILE *realf, *tmpf; int tmpres; struct stat statb; bool result, exists; @@ -3989,6 +3988,7 @@ DBCellWrite(cellDef, fileName) tmpname = StrDup((char **)NULL, expandname); } + FILE *realf = NULL, *tmpf; /* * See if we can create a temporary file in this directory. * If so, write to the temp file and then rename it after @@ -4119,6 +4119,7 @@ DBCellWrite(cellDef, fileName) #endif realf = fopen(expandname, "r"); + bool do_close = FALSE; if (realf == NULL) { cellDef->cd_flags |= CDMODIFIED; @@ -4135,15 +4136,26 @@ DBCellWrite(cellDef, fileName) } #ifdef FILE_LOCKS + /* when file locking is in use the FD needs to stay open to hold the lock + * as with fcntl() locking any call to close() on any FD even dup() and + * those from separate open() calls, will cause all locks to be dropped + * by all FDs as they are process wide locks and associated with file + * system st_dev(kernel-device)/st_ino(inode) and not with FD handles. + */ cellDef->cd_fd = -1; if (FileLocking && (is_locked == FALSE)) cellDef->cd_fd = fd; else if (FileLocking && (is_locked == TRUE)) cellDef->cd_fd = -2; else + do_close = TRUE; +#else + do_close = TRUE; #endif - fclose(realf); } + if(do_close) + fclose(realf); + /* invalidate even if we don't close to ensure cleanup below does not close */ realf = NULL; } @@ -4151,6 +4163,8 @@ cleanup: SigEnableInterrupts(); freeMagic(realname); freeMagic(tmpname); + if(realf) + fclose(realf); return result; }