Merge pull request #79 from bbc/philipn-fixes-from-fsanitize

Add some fixes found when building with -fsanitize
bbc · Jun 14, 2024 · afe0b7c · afe0b7c
2 parents fe923a9 + d5ac6c5
commit afe0b7c
Show file tree

Hide file tree

Showing 5 changed files with 81 additions and 24 deletions.
diff --git a/deps/libMXF/mxf/mxf_file.c b/deps/libMXF/mxf/mxf_file.c
@@ -123,18 +123,36 @@ static void disk_file_close(MXFFileSysData *sysData)
 static uint32_t disk_file_read(MXFFileSysData *sysData, uint8_t *data, uint32_t count)
 {
     char errorBuf[128];
-    uint32_t result = (uint32_t)fread(data, 1, count, sysData->file);
-    if (result != count && ferror(sysData->file))
-        mxf_log_error("fread failed: %s\n", mxf_strerror(errno, errorBuf, sizeof(errorBuf)));
+    uint32_t result;
+
+    if (data) {
+        result = (uint32_t)fread(data, 1, count, sysData->file);
+        if (result != count && ferror(sysData->file))
+            mxf_log_error("fread failed: %s\n", mxf_strerror(errno, errorBuf, sizeof(errorBuf)));
+    } else {
+        result = 0;
+        if (result != count)
+            mxf_log_error("fread failed: passed NULL data ptr\n");
+    }
+
     return result;
 }
 
 static uint32_t disk_file_write(MXFFileSysData *sysData, const uint8_t *data, uint32_t count)
 {
     char errorBuf[128];
-    uint32_t result = (uint32_t)fwrite(data, 1, count, sysData->file);
-    if (result != count)
-        mxf_log_error("fwrite failed: %s\n", mxf_strerror(errno, errorBuf, sizeof(errorBuf)));
+    uint32_t result;
+
+    if (data) {
+        result = (uint32_t)fwrite(data, 1, count, sysData->file);
+        if (result != count)
+            mxf_log_error("fwrite failed: %s\n", mxf_strerror(errno, errorBuf, sizeof(errorBuf)));
+    } else {
+        result = 0;
+        if (result != count)
+            mxf_log_error("fwrite failed: passed NULL data ptr\n");
+    }
+
     return result;
 }
 
@@ -422,10 +440,10 @@ int mxf_read_uint32(MXFFile *mxfFile, uint32_t *value)
     uint8_t buffer[4];
     CHK_ORET(mxf_file_read(mxfFile, buffer, 4) == 4);
 
-    *value = (buffer[0] << 24) |
-             (buffer[1] << 16) |
-             (buffer[2] << 8) |
-              buffer[3];
+    *value = ((uint32_t)buffer[0] << 24) |
+             ((uint32_t)buffer[1] << 16) |
+             ((uint32_t)buffer[2] << 8) |
+              (uint32_t)buffer[3];
 
     return 1;
 }

diff --git a/deps/libMXF/mxf/mxf_page_file.c b/deps/libMXF/mxf/mxf_page_file.c
@@ -129,18 +129,36 @@ static void disk_file_close(FileDescriptor *fileDesc)
 static uint32_t disk_file_read(FileDescriptor *fileDesc, uint8_t *data, uint32_t count)
 {
     char errorBuf[128];
-    uint32_t result = (uint32_t)fread(data, 1, count, fileDesc->file);
-    if (result != count && ferror(fileDesc->file))
-        mxf_log_error("fread failed: %s\n", mxf_strerror(errno, errorBuf, sizeof(errorBuf)));
+    uint32_t result;
+
+    if (data) {
+        result = (uint32_t)fread(data, 1, count, fileDesc->file);
+        if (result != count && ferror(fileDesc->file))
+            mxf_log_error("fread failed: %s\n", mxf_strerror(errno, errorBuf, sizeof(errorBuf)));
+    } else {
+        result = 0;
+        if (result != count)
+            mxf_log_error("fread failed: passed NULL data ptr\n");
+    }
+
     return result;
 }
 
 static uint32_t disk_file_write(FileDescriptor *fileDesc, const uint8_t *data, uint32_t count)
 {
     char errorBuf[128];
-    uint32_t result = (uint32_t)fwrite(data, 1, count, fileDesc->file);
-    if (result != count)
-        mxf_log_error("fwrite failed: %s\n", mxf_strerror(errno, errorBuf, sizeof(errorBuf)));
+    uint32_t result;
+
+    if (data) {
+        result = (uint32_t)fwrite(data, 1, count, fileDesc->file);
+        if (result != count)
+            mxf_log_error("fwrite failed: %s\n", mxf_strerror(errno, errorBuf, sizeof(errorBuf)));
+    } else {
+        result = 0;
+        if (result != count)
+            mxf_log_error("fwrite failed: passed NULL data ptr\n");
+    }
+
     return result;
 }
 
@@ -342,7 +360,8 @@ static Page* open_page(MXFFileSysData *sysData, int64_t position)
 
             Page *newPages;
             CHK_MALLOC_ARRAY_ORET(newPages, Page, sysData->numPagesAllocated + PAGE_ALLOC_INCR);
-            memcpy(newPages, sysData->pages, sizeof(Page) * sysData->numPagesAllocated);
+            if (sysData->pages)
+                memcpy(newPages, sysData->pages, sizeof(Page) * sysData->numPagesAllocated);
             SAFE_FREE(sysData->pages);
             sysData->pages = newPages;
             sysData->numPagesAllocated += PAGE_ALLOC_INCR;

diff --git a/deps/libMXFpp/libMXF++/HeaderMetadata.cpp b/deps/libMXFpp/libMXF++/HeaderMetadata.cpp
@@ -117,8 +117,12 @@ void HeaderMetadata::registerObjectFactory(const mxfKey *key, AbsMetadataSetFact
         _objectFactory.insert(pair<mxfKey, AbsMetadataSetFactory*>(*key, factory));
     if (result.second == false)
     {
-        // replace existing factory with new one
+        // erase and delete the existing factory
+        AbsMetadataSetFactory *existing_factory = result.first->second;
         _objectFactory.erase(result.first);
+        delete existing_factory;
+
+        // insert the new factory
         _objectFactory.insert(pair<mxfKey, AbsMetadataSetFactory*>(*key, factory));
     }
 }

diff --git a/src/common/BMXFileIO.cpp b/src/common/BMXFileIO.cpp
@@ -80,9 +80,16 @@ BMXFileIO::~BMXFileIO()
 
 uint32_t BMXFileIO::Read(unsigned char *data, uint32_t size)
 {
-    size_t result = fread(data, 1, size, mFile);
-    if (result != size && ferror(mFile))
-        log_error("Failed to read %u bytes: %s\n", size, bmx_strerror(errno).c_str());
+    size_t result;
+    if (data) {
+        result = fread(data, 1, size, mFile);
+        if (result != size && ferror(mFile))
+            log_error("Failed to read %u bytes: %s\n", size, bmx_strerror(errno).c_str());
+    } else {
+        result = 0;
+        if (result != size)
+            log_error("Failed to read %u bytes: passed NULL data ptr\n", size);
+    }
 
     return (uint32_t)result;
 }
@@ -91,9 +98,16 @@ uint32_t BMXFileIO::Write(const unsigned char *data, uint32_t size)
 {
     BMX_ASSERT(!mReadOnly);
 
-    size_t result = fwrite(data, 1, size, mFile);
-    if (result != size)
-        log_error("Failed to write %u bytes: %s\n", size, bmx_strerror(errno).c_str());
+    size_t result;
+    if (data) {
+        result = fwrite(data, 1, size, mFile);
+        if (result != size)
+            log_error("Failed to write %u bytes: %s\n", size, bmx_strerror(errno).c_str());
+    } else {
+        result = 0;
+        if (result != size)
+            log_error("Failed to write %u bytes: passed NULL data ptr\n", size);
+    }
 
     return (uint32_t)result;
 }

diff --git a/test/create_test_essence.cpp b/test/create_test_essence.cpp
@@ -430,6 +430,8 @@ static void write_d10(FILE *file, int type, unsigned int duration)
     unsigned int i;
     for (i = 0; i < duration; i++)
         write_buffer(file, data, frame_size);
+
+    delete [] data;
 }
 
 static void write_mpeg2lg(FILE *file, int type, unsigned int duration, bool low_delay, bool closed_gop,