Address CodeQL issues

.github/codeql-config.yml

@@ -0,0 +1,17 @@
+# Query filters to include or exclude specific queries
+query-filters:
+  - exclude:
+      # See: https://codeql.github.com/codeql-query-help/cpp/cpp-short-global-name/
+      id: cpp/short-global-name
+  - exclude:
+      # See: https://codeql.github.com/codeql-query-help/cpp/cpp-commented-out-code/ 
+      id: cpp/commented-out-code
+  - exclude:
+      # See: https://codeql.github.com/codeql-query-help/cpp/cpp-poorly-documented-function/
+      id: cpp/poorly-documented-function
+  - exclude:
+      # See: https://codeql.github.com/codeql-query-help/cpp/cpp-trivial-switch/
+      id: cpp/trivial-switch
+  - exclude:
+      # See: https://codeql.github.com/codeql-query-help/cpp/cpp-irregular-enum-init/
+      id: cpp/irregular-enum-init

.github/workflows/codeql.yml

@@ -198,3 +198,26 @@ jobs:
         uses: github/codeql-action/analyze@v3
         with:
           category: "/language:c-cpp"
+          output: sarif-results
+          upload: failure-only
+
+      - name: filter-sarif
+        uses: advanced-security/filter-sarif@main
+        with:
+          patterns: |
+            -**/*
+            src/**/*
+          input: sarif-results/cpp.sarif
+          output: sarif-results/cpp.sarif
+
+      - name: Upload SARIF
+        uses: github/codeql-action/upload-sarif@v3
+        with:
+          sarif_file: sarif-results/cpp.sarif
+
+      - name: Upload loc as a Build Artifact
+        uses: actions/upload-artifact@v4
+        with:
+          name: sarif-results
+          path: sarif-results
+          retention-days: 1

.github/workflows/linux.yml

@@ -16,7 +16,7 @@ on:
 jobs:
   vol-cache:
     runs-on: ubuntu-latest
-    timeout-minutes: 60
+    timeout-minutes: 20
 
     steps:
       - uses: actions/checkout@v4.1.1
@@ -60,7 +60,7 @@ jobs:
           # Compile HDF5
           mkdir -p hdf5/build
           cd hdf5/build
-          cmake -DCMAKE_BUILD_TYPE=Release -DCMAKE_INSTALL_PREFIX=$HDF5_DIR -DHDF5_ENABLE_PARALLEL:BOOL=ON -DHDF5_ENABLE_THREADSAFE:BOOL=ON -DALLOW_UNSUPPORTED:BOOL=ON ..
+          cmake -DCMAKE_BUILD_TYPE=Release -DCMAKE_INSTALL_PREFIX=$HDF5_DIR -DHDF5_ENABLE_PARALLEL:BOOL=ON -DHDF5_ENABLE_THREADSAFE:BOOL=ON -DHDF5_ALLOW_UNSUPPORTED:BOOL=ON ..
           make -j2 install
           cd -
           cd argobots

src/H5LS.c

@@ -144,7 +144,7 @@ herr_t readLSConf(char *fname, cache_storage_t *LS) {
     linenum++;
     if (line[0] == '#')
       continue;
-    if (sscanf(line, "%[^:]:%s", ip, mac) != 2) {
+    if (sscanf(line, "%[^:]:%255s", ip, mac) != 2) {
       if (RANK == io_node())
         fprintf(stderr, "Syntax error, line %d\n", linenum);
       continue;
@@ -153,7 +153,7 @@ herr_t readLSConf(char *fname, cache_storage_t *LS) {
       if (strcmp(mac, "NULL") == 0)
         LS->path = NULL;
       else {
-        strcpy(LS->path, mac);
+        snprintf(LS->path, 255, "%s", mac);
       }
 
     else if (!strcmp(ip, "HDF5_CACHE_FUSION_THRESHOLD")) {

src/H5VLcache_ext.c

@@ -44,6 +44,7 @@
 #include <sys/statvfs.h>
 #include <unistd.h>
 // debug
+#define LOG_BUFFER_SIZE 1024
 // VOL related header
 #include "H5LS.h"
 #include "H5VLcache_ext_private.h"
@@ -96,6 +97,7 @@
 int RANK = 0;
 int NPROC = 1;
 hbool_t HDF5_CACHE_CLOSE_ASYNC = 0;
+char log_buffer[LOG_BUFFER_SIZE];
 // Functions from async VOL
 int H5VL_async_set_delay_time(uint64_t time_us);
 herr_t H5VL_async_set_request_dep(void *request, void *parent_request);
@@ -106,7 +108,9 @@
 
 #define H5Pcopy(X)                                                             \
   H5Pcopy(X);                                                                  \
-  LOG_DEBUG(-1, "H5Pcopy called: %s:%d %s\n", __FILE__, __LINE__, __FUNCTION__);
+  snprintf(log_buffer, LOG_BUFFER_SIZE, "H5Pcopy called: %s:%d %s\n",          \
+           __FILE__, __LINE__, __FUNCTION__);                                  \
+  LOG_DEBUG(-1, "%s", log_buffer);
 
 #define H5Scopy(X)                                                             \
   H5Scopy(X);                                                                  \
@@ -496,6 +500,8 @@
     return t->remove_file_cache(obj, req);
   else if (o->obj_type == H5I_DATASET)
     return t->remove_dataset_cache(obj, req);
+  else
+    LOG_ERROR(-1, "Unknown object type for cache removal");
 }
 
 static herr_t create_cache(void *obj, void *arg, void **req) {
@@ -511,6 +517,8 @@
     return t->create_file_cache(obj, arg, req);
   else if (o->obj_type == H5I_DATASET)
     return t->create_dataset_cache(obj, arg, req);
+  else
+    LOG_ERROR(-1, "Unknown object type for cache creation");
 }
 /*******************/
 /* Local variables */
@@ -770,19 +778,20 @@
     LOG_WARN(-1, "Close request is NULL.");
   }
 #ifndef NDEBUG
-  LOG_DEBUG(-1, "async task finished %d", task->type);
+  snprintf(log_buffer, LOG_BUFFER_SIZE, "async task finished %d", task->type);
+  LOG_DEBUG(-1, "%s", log_buffer);
   double t1 = MPI_Wtime();
-  LOG_DEBUG(-1,
-            "Delay closed object: %d time: "
-            "%10.6f",
-            task->type, t1 - t0);
+  snprintf(log_buffer, LOG_BUFFER_SIZE, "Delay closed object: %d time: %10.6f",
+           task->type, t1 - t0);
+  LOG_DEBUG(-1, "%s", log_buffer);
 #endif
   if (o->read_cache || o->write_cache)
     o->H5LS->cache_io_cls->remove_cache(task->obj, NULL);
   H5VL_cache_ext_free_obj(o);
 #ifndef NDEBUG
   double t2 = MPI_Wtime();
-  LOG_DEBUG(-1, "Remove cache time: %10.6f", t2 - t1);
+  snprintf(log_buffer, LOG_BUFFER_SIZE, "Remove cache time: %10.6f", t2 - t1);
+  LOG_DEBUG(-1, "%s", log_buffer);
 #endif
   free(task->req);
   return 0;
@@ -1422,11 +1431,22 @@
 
   LOG_INFO(-1, "       storage path: %s", p->H5LS->path);
 
-  LOG_INFO(-1, "       storage size: %.4f GiB",
-           p->H5LS->mspace_total / 1024. / 1024. / 1024.);
+  int ret =
+      snprintf(log_buffer, LOG_BUFFER_SIZE, "       storage size: %.4f GiB",
+               p->H5LS->mspace_total / 1024. / 1024. / 1024.);
+  if (ret < 0 || ret >= LOG_BUFFER_SIZE) {
+    LOG_WARN(-1, "Log Error when formatting storage size message");
+  } else {
+    LOG_INFO(-1, "%s", log_buffer);
+  }
 
-  LOG_INFO(-1, "  write buffer size: %.4f GiB",
-           p->H5LS->write_buffer_size / 1024. / 1024. / 1024.);
+  ret = snprintf(log_buffer, LOG_BUFFER_SIZE, "  write buffer size: %.4f GiB",
+                 p->H5LS->write_buffer_size / 1024. / 1024. / 1024.);
+  if (ret < 0 || ret >= LOG_BUFFER_SIZE) {
+    LOG_WARN(-1, "Log Error when formatting write buffer size message");
+  } else {
+    LOG_INFO(-1, "%s", log_buffer);
+  }
 
   LOG_INFO(-1, "       storage type: %s", p->H5LS->type);
 
@@ -2539,11 +2559,12 @@
   }
   H5VL_request_status_t status;
 #ifndef NDEBUG
-  LOG_DEBUG(-1,
-            "request wait(jobid: %d), current available space: "
-            "%.5f GiB ",
-            o->H5DWMM->io->current_request->id,
-            o->H5DWMM->cache->mspace_per_rank_left / 1024. / 1024. / 1024);
+  snprintf(log_buffer, LOG_BUFFER_SIZE,
+           "request wait(jobid: %d), current available space: "
+           "%.5f GiB ",
+           o->H5DWMM->io->current_request->id,
+           o->H5DWMM->cache->mspace_per_rank_left / 1024. / 1024. / 1024);
+  LOG_DEBUG(-1, "%s", log_buffer);
 #endif
   while ((o->H5DWMM->io->current_request != NULL &&
           o->H5DWMM->io->current_request->req != NULL)) {
@@ -2627,8 +2648,7 @@
                              // nearby write requests.
   t_com->id = r->id;
 #ifndef NDEBUG
-
-  LOG_DEBUG(-1, "Merging %d tasks (%d - %d) ", ntasks, t_com->id,
+  LOG_DEBUG(-1, "Merging %d tasks (%d - %d)", ntasks, t_com->id,
             t_com->id + ntasks - 1);
 
 #endif
@@ -2657,8 +2677,8 @@
   free(t_com);
   double t1 = MPI_Wtime();
 #ifndef NDEBUG
-  LOG_DEBUG(-1, "Merging time: %6.5f", t1 - t0);
-
+  snprintf(log_buffer, LOG_BUFFER_SIZE, "Merging time: %6.5f", t1 - t0);
+  LOG_DEBUG(-1, "%s", log_buffer);
 #endif
   return SUCCEED;
 }
@@ -3066,8 +3086,10 @@
       }
       double t1 = MPI_Wtime();
 #ifndef NDEBUG
-      LOG_DEBUG(-1, "H5VLreqeust_wait time (jobid: %d): %f",
-                o->H5DWMM->io->current_request->id, t1 - t0);
+      snprintf(log_buffer, LOG_BUFFER_SIZE,
+               "H5VLreqeust_wait time (jobid: %d): %g",
+               o->H5DWMM->io->current_request->id, t1 - t0);
+      LOG_DEBUG(-1, "%s", log_buffer);
 
       LOG_DEBUG(-1, "Tasks %d(%ld merged) finished",
                 o->H5DWMM->io->current_request->id,
@@ -3218,11 +3240,11 @@
     p->async_close_task_list->obj = NULL;
     double t1 = MPI_Wtime();
 #ifndef NDEBUG
-
-    LOG_DEBUG(-1,
-              "dataset close time: "
-              "%.6f seconds",
-              t1 - t0);
+    snprintf(log_buffer, LOG_BUFFER_SIZE,
+             "dataset close time: "
+             "%.6f seconds",
+             t1 - t0);
+    LOG_DEBUG(-1, "%s", log_buffer);
 
 #endif
     return ret_value;
@@ -3234,10 +3256,11 @@
     double t1 = MPI_Wtime();
 #ifndef NDEBUG
 
-    LOG_DEBUG(-1,
-              "dataset remove cache time (including wait time): "
-              "%.6f seconds",
-              t1 - t0);
+    snprintf(log_buffer, LOG_BUFFER_SIZE,
+             "dataset remove cache time (including wait time): "
+             "%.6f seconds",
+             t1 - t0);
+    LOG_DEBUG(-1, "%s", log_buffer);
 
 #endif
   }
@@ -3259,7 +3282,9 @@
     H5VL_cache_ext_free_obj(o);
   double tt1 = MPI_Wtime();
 #ifndef NDEBUG
-  LOG_DEBUG(-1, "H5VL_cache_ext_dataset_close time: %.6f seconds", tt1 - tt0);
+  snprintf(log_buffer, LOG_BUFFER_SIZE,
+           "H5VL_cache_ext_dataset_close time: %.6f seconds", tt1 - tt0);
+  LOG_DEBUG(-1, "%s", log_buffer);
 
 #endif
   return ret_value;
@@ -5649,15 +5674,30 @@
 
       if (dset->H5LS->path != NULL) {
         strcpy(dset->H5DRMM->cache->path, p->H5DRMM->cache->path); // create
-        strcat(dset->H5DRMM->cache->path, "/");
-        strcat(dset->H5DRMM->cache->path, name);
-        strcat(dset->H5DRMM->cache->path, "/");
-        strcpy(dset->H5DRMM->mmap->fname, dset->H5DRMM->cache->path);
-        strcat(dset->H5DRMM->mmap->fname, "/dset-mmap-");
+        strncat(dset->H5DRMM->cache->path, "/",
+                sizeof(dset->H5DRMM->cache->path) -
+                    strlen(dset->H5DRMM->cache->path) - 1);
+        strncat(dset->H5DRMM->cache->path, name,
+                sizeof(dset->H5DRMM->cache->path) -
+                    strlen(dset->H5DRMM->cache->path) - 1);
+        strncat(dset->H5DRMM->cache->path, "/",
+                sizeof(dset->H5DRMM->cache->path) -
+                    strlen(dset->H5DRMM->cache->path) - 1);
+        strncpy(dset->H5DRMM->mmap->fname, dset->H5DRMM->cache->path,
+                sizeof(dset->H5DRMM->mmap->fname) - 1);
+        dset->H5DRMM->mmap->fname[sizeof(dset->H5DRMM->mmap->fname) - 1] =
+            '\0'; // Ensure null-termination
+        strncat(dset->H5DRMM->mmap->fname, "/dset-mmap-",
+                sizeof(dset->H5DRMM->mmap->fname) -
+                    strlen(dset->H5DRMM->mmap->fname) - 1);
         char cc[255];
         int2char(dset->H5DRMM->mpi->rank, cc);
-        strcat(dset->H5DRMM->mmap->fname, cc);
-        strcat(dset->H5DRMM->mmap->fname, ".dat");
+        strncat(dset->H5DRMM->mmap->fname, cc,
+                sizeof(dset->H5DRMM->mmap->fname) -
+                    strlen(dset->H5DRMM->mmap->fname) - 1);
+        strncat(dset->H5DRMM->mmap->fname, ".dat",
+                sizeof(dset->H5DRMM->mmap->fname) -
+                    strlen(dset->H5DRMM->mmap->fname) - 1);
 #ifndef NDEBUG
 
         LOG_DEBUG(-1, "Dataset read cache created: %s",
@@ -5735,9 +5775,15 @@
     memcpy(group->H5DRMM->mpi, o->H5DRMM->mpi, sizeof(MPI_INFO));
     if (group->H5LS->path != NULL) {
       strcpy(group->H5DRMM->cache->path, o->H5DRMM->cache->path); // create
-      strcat(group->H5DRMM->cache->path, "/");
-      strcat(group->H5DRMM->cache->path, name);
-      strcat(group->H5DRMM->cache->path, "/");
+      size_t remaining_size = sizeof(group->H5DRMM->cache->path) -
+                              strlen(group->H5DRMM->cache->path) - 1;
+      strncat(group->H5DRMM->cache->path, "/", remaining_size);
+      remaining_size = sizeof(group->H5DRMM->cache->path) -
+                       strlen(group->H5DRMM->cache->path) - 1;
+      strncat(group->H5DRMM->cache->path, name, remaining_size);
+      remaining_size = sizeof(group->H5DRMM->cache->path) -
+                       strlen(group->H5DRMM->cache->path) - 1;
+      strncat(group->H5DRMM->cache->path, "/", remaining_size);
 #ifndef NDEBUG
       LOG_DEBUG(-1, "group cache created: %s", group->H5DRMM->cache->path);
 #endif
@@ -5790,7 +5836,8 @@
     H5VL_cache_ext_dataset_wait(dset);
     double t1 = MPI_Wtime();
 #ifndef NDEBUG
-    LOG_DEBUG(-1, "dataset_wait time: %f", t1 - t0);
+    snprintf(log_buffer, LOG_BUFFER_SIZE, "dataset_wait time: %f", t1 - t0);
+    LOG_DEBUG(-1, "%s", log_buffer);
 #endif
     o->H5DWMM = NULL;
   }