Replace use of Variable Length Arrays in C code (#3323)

* CI: add -Wvla
* Replace use of Variable Length Arrays
* use sizeof(int)

.github/workflows/macos_install.sh

@@ -66,7 +66,7 @@ CONFIGURE_FLAGS="\
   --with-readline-libs=${CONDA_PREFIX}/lib
 "
 
-export CFLAGS="-O2 -pipe -arch ${CONDA_ARCH} -DGL_SILENCE_DEPRECATION -Wall -Wextra -Wpedantic"
+export CFLAGS="-O2 -pipe -arch ${CONDA_ARCH} -DGL_SILENCE_DEPRECATION -Wall -Wextra -Wpedantic -Wvla"
 export CXXFLAGS="-O2 -pipe -stdlib=libc++ -arch ${CONDA_ARCH} -Wall -Wextra -Wpedantic"
 export CPPFLAGS="-isystem${CONDA_PREFIX}/include"
 

.github/workflows/ubuntu.yml

@@ -69,7 +69,7 @@ jobs:
       - name: Build
         env:
           # TODO: -pedantic-errors here won't go through ./configure (with GNU C)
-          CFLAGS: -fPIC
+          CFLAGS: -fPIC -Wvla
           # TODO: -pedantic-errors here won't compile
           CXXFLAGS: -fPIC
         run: .github/workflows/build_${{ matrix.config }}.sh $HOME/install -Werror

imagery/i.landsat.acca/algorithm.c

@@ -75,9 +75,12 @@ extern int hist_n;
 void acca_algorithm(Gfile *out, Gfile band[], int single_pass, int with_shadow,
                     int cloud_signature)
 {
-    int i, count[5], hist_cold[hist_n], hist_warm[hist_n];
+    int i, count[5];
     double max, value[5], signa[5], idesert, review_warm, shift;
 
+    int *hist_cold = G_malloc(hist_n * sizeof(int));
+    int *hist_warm = G_malloc(hist_n * sizeof(int));
+
     /* Reset variables ... */
     for (i = 0; i < 5; i++) {
         count[i] = 0;
@@ -213,6 +216,9 @@ void acca_algorithm(Gfile *out, Gfile band[], int single_pass, int with_shadow,
     acca_second(out, band[BAND6], review_warm, value[KUPPER], value[KLOWER]);
     /* CATEGORIES: IS_WARM_CLOUD, IS_COLD_CLOUD, IS_SHADOW, NULL (= NO_CLOUD) */
 
+    G_free(hist_cold);
+    G_free(hist_warm);
+
     return;
 }
 

lib/gis/strings.c

@@ -267,17 +267,16 @@ char *G_str_replace(const char *buffer, const char *old_str,
 char *G_str_concat(const char **src_strings, int num_strings, const char *sep,
                    int maxsize)
 {
-    char buffer[maxsize];
-    int i;
-    char *end = buffer + maxsize;
-    char *p = NULL;
-
     if (maxsize < 1 || num_strings < 1)
         return NULL;
 
-    memset(buffer, 0, sizeof(buffer));
+    char *concat_str = NULL;
+    char *p = NULL;
+    char *buffer = G_malloc(maxsize * sizeof(char));
+    char *end = buffer + maxsize;
 
-    for (i = 0; i < num_strings; i++) {
+    memset(buffer, 0, maxsize);
+    for (int i = 0; i < num_strings; i++) {
         if (i == 0)
             p = (char *)G__memccpy(buffer, src_strings[i], '\0', maxsize);
         else {
@@ -287,8 +286,10 @@ char *G_str_concat(const char **src_strings, int num_strings, const char *sep,
                 p = (char *)G__memccpy(p - 1, src_strings[i], '\0', end - p);
         }
     }
+    concat_str = G_store(buffer);
+    G_free(buffer);
 
-    return G_store(buffer);
+    return concat_str;
 }
 
 /*!

lib/imagery/iscatt_core.c

@@ -628,29 +628,32 @@ static void get_needed_bands(struct scCats *cats, int *b_needed_bands)
  */
 static void free_compute_scatts_data(int *fd_bands, struct rast_row *bands_rows,
                                      int n_a_bands, int *bands_ids,
-                                     int *fd_cats_rasts,
+                                     int *b_needed_bands, int *fd_cats_rasts,
                                      FILE **f_cats_rasts_conds, int n_a_cats)
 {
-    int i, band_id;
-
-    for (i = 0; i < n_a_bands; i++) {
-        band_id = bands_ids[i];
+    for (int i = 0; i < n_a_bands; i++) {
+        int band_id = bands_ids[i];
         if (band_id >= 0) {
             Rast_close(fd_bands[i]);
             G_free(bands_rows[band_id].row);
             G_free(bands_rows[band_id].null_row);
         }
     }
 
-    if (f_cats_rasts_conds)
-        for (i = 0; i < n_a_cats; i++)
-            if (f_cats_rasts_conds[i])
-                fclose(f_cats_rasts_conds[i]);
+    for (int i = 0; i < n_a_cats; i++)
+        if (f_cats_rasts_conds[i])
+            fclose(f_cats_rasts_conds[i]);
+
+    for (int i = 0; i < n_a_cats; i++)
+        if (fd_cats_rasts[i] >= 0)
+            Rast_close(fd_cats_rasts[i]);
 
-    if (fd_cats_rasts)
-        for (i = 0; i < n_a_cats; i++)
-            if (fd_cats_rasts[i] >= 0)
-                Rast_close(fd_cats_rasts[i]);
+    G_free(fd_bands);
+    G_free(bands_rows);
+    G_free(bands_ids);
+    G_free(b_needed_bands);
+    G_free(fd_cats_rasts);
+    G_free(f_cats_rasts_conds);
 }
 
 /*!
@@ -687,19 +690,20 @@ int I_compute_scatts(struct Cell_head *region, struct scCats *scatt_conds,
     const char *mapset;
     char header[1024];
 
-    int fd_cats_rasts[scatt_conds->n_a_cats];
-    FILE *f_cats_rasts_conds[scatt_conds->n_a_cats];
+    int *fd_cats_rasts = G_malloc(scatt_conds->n_a_cats * sizeof(int));
+    FILE **f_cats_rasts_conds =
+        G_malloc(scatt_conds->n_a_cats * sizeof(FILE *));
 
-    struct rast_row bands_rows[n_bands];
+    struct rast_row *bands_rows = G_malloc(n_bands * sizeof(struct rast_row));
 
     RASTER_MAP_TYPE data_type;
 
     int nrows, i_band, n_a_bands, band_id;
     int i_row, head_nchars, i_cat, id_cat;
 
-    int fd_bands[n_bands];
-    int bands_ids[n_bands];
-    int b_needed_bands[n_bands];
+    int *fd_bands = G_malloc(n_bands * sizeof(int));
+    int *bands_ids = G_malloc(n_bands * sizeof(int));
+    int *b_needed_bands = G_malloc(n_bands * sizeof(int));
 
     Rast_set_window(region);
 
@@ -712,6 +716,9 @@ int I_compute_scatts(struct Cell_head *region, struct scCats *scatt_conds,
     if (n_bands != scatts->n_bands || n_bands != scatt_conds->n_bands)
         return -1;
 
+    for (i_cat = 0; i_cat < scatts->n_a_cats; i_cat++)
+        fd_cats_rasts[i_cat] = -1;
+
     G_zero(b_needed_bands, (size_t)n_bands * sizeof(int));
 
     get_needed_bands(scatt_conds, &b_needed_bands[0]);
@@ -726,18 +733,18 @@ int I_compute_scatts(struct Cell_head *region, struct scCats *scatt_conds,
                     bands[band_id]);
 
             if ((mapset = G_find_raster2(bands[band_id], "")) == NULL) {
-                free_compute_scatts_data(fd_bands, bands_rows, n_a_bands,
-                                         bands_ids, NULL, NULL,
-                                         scatt_conds->n_a_cats);
+                free_compute_scatts_data(
+                    fd_bands, bands_rows, n_a_bands, bands_ids, b_needed_bands,
+                    fd_cats_rasts, f_cats_rasts_conds, scatt_conds->n_a_cats);
                 G_warning(_("Unable to find raster <%s>"), bands[band_id]);
                 return -1;
             }
 
             if ((fd_bands[n_a_bands] = Rast_open_old(bands[band_id], mapset)) <
                 0) {
-                free_compute_scatts_data(fd_bands, bands_rows, n_a_bands,
-                                         bands_ids, NULL, NULL,
-                                         scatt_conds->n_a_cats);
+                free_compute_scatts_data(
+                    fd_bands, bands_rows, n_a_bands, bands_ids, b_needed_bands,
+                    fd_cats_rasts, f_cats_rasts_conds, scatt_conds->n_a_cats);
                 G_warning(_("Unable to open raster <%s>"), bands[band_id]);
                 return -1;
             }
@@ -754,9 +761,9 @@ int I_compute_scatts(struct Cell_head *region, struct scCats *scatt_conds,
 
             if (Rast_read_range(bands[band_id], mapset,
                                 &bands_rows[band_id].rast_range) != 1) {
-                free_compute_scatts_data(fd_bands, bands_rows, n_a_bands,
-                                         bands_ids, NULL, NULL,
-                                         scatt_conds->n_a_cats);
+                free_compute_scatts_data(
+                    fd_bands, bands_rows, n_a_bands, bands_ids, b_needed_bands,
+                    fd_cats_rasts, f_cats_rasts_conds, scatt_conds->n_a_cats);
                 G_warning(_("Unable to read range of raster <%s>"),
                           bands[band_id]);
                 return -1;
@@ -773,15 +780,13 @@ int I_compute_scatts(struct Cell_head *region, struct scCats *scatt_conds,
         if (cats_rasts[id_cat]) {
             fd_cats_rasts[i_cat] = Rast_open_new(cats_rasts[id_cat], CELL_TYPE);
         }
-        else
-            fd_cats_rasts[i_cat] = -1;
 
         if (cats_rasts_conds[id_cat]) {
             f_cats_rasts_conds[i_cat] = fopen(cats_rasts_conds[id_cat], "r");
             if (!f_cats_rasts_conds[i_cat]) {
                 free_compute_scatts_data(
-                    fd_bands, bands_rows, n_a_bands, bands_ids, fd_cats_rasts,
-                    f_cats_rasts_conds, scatt_conds->n_a_cats);
+                    fd_bands, bands_rows, n_a_bands, bands_ids, b_needed_bands,
+                    fd_cats_rasts, f_cats_rasts_conds, scatt_conds->n_a_cats);
                 G_warning(
                     _("Unable to open category raster condition file <%s>"),
                     bands[band_id]);
@@ -815,13 +820,13 @@ int I_compute_scatts(struct Cell_head *region, struct scCats *scatt_conds,
                                           bands_rows, scatts,
                                           fd_cats_rasts) == -1) {
             free_compute_scatts_data(fd_bands, bands_rows, n_a_bands, bands_ids,
-                                     fd_cats_rasts, f_cats_rasts_conds,
-                                     scatt_conds->n_a_cats);
+                                     b_needed_bands, fd_cats_rasts,
+                                     f_cats_rasts_conds, scatt_conds->n_a_cats);
             return -1;
         }
     }
     free_compute_scatts_data(fd_bands, bands_rows, n_a_bands, bands_ids,
-                             fd_cats_rasts, f_cats_rasts_conds,
+                             b_needed_bands, fd_cats_rasts, f_cats_rasts_conds,
                              scatt_conds->n_a_cats);
     return 0;
 }

raster/r.fill.stats/main.c

@@ -147,6 +147,8 @@ long int estimate_mem_needed(long int cols, char *mode)
     return (mem_count);
 }
 
+#define WEIGHT_MATRIX_LINE_LENGTH 80
+
 /*
  * Prints the spatial weights matrix to the console.
  * This uses a fixed layout which may not be able to print very
@@ -155,27 +157,26 @@ long int estimate_mem_needed(long int cols, char *mode)
 void print_weights_matrix(long int rows, long int cols)
 {
     int i, j;
-    size_t weight_matrix_line_length = 80;
-    char weight_matrix_line_buf[weight_matrix_line_length + 1];
-    char weight_matrix_weight_buf[weight_matrix_line_length + 1];
+    char weight_matrix_line_buf[WEIGHT_MATRIX_LINE_LENGTH + 1];
+    char weight_matrix_weight_buf[WEIGHT_MATRIX_LINE_LENGTH + 1];
 
     G_message(_("Spatial weights neighborhood (cells):"));
     for (i = 0; i < rows; i++) {
         weight_matrix_line_buf[0] = '\0';
         for (j = 0; j < cols; j++) {
             if (WEIGHTS[i][j] != -1.0) {
-                snprintf(weight_matrix_weight_buf, weight_matrix_line_length,
+                snprintf(weight_matrix_weight_buf, WEIGHT_MATRIX_LINE_LENGTH,
                          "%06.2f ", WEIGHTS[i][j]);
             }
             else {
-                snprintf(weight_matrix_weight_buf, weight_matrix_line_length,
+                snprintf(weight_matrix_weight_buf, WEIGHT_MATRIX_LINE_LENGTH,
                          "...... ");
             }
             if (strlen(weight_matrix_weight_buf) +
                     strlen(weight_matrix_line_buf) >
-                weight_matrix_line_length) {
+                WEIGHT_MATRIX_LINE_LENGTH) {
                 strncpy(weight_matrix_line_buf, "[line too long to print]",
-                        weight_matrix_line_length);
+                        WEIGHT_MATRIX_LINE_LENGTH);
                 break;
             }
             else {

raster/r.mapcalc/map.c

@@ -725,9 +725,9 @@ void copy_history(const char *dst, int idx)
     Rast_write_history((char *)dst, &hist);
 }
 
+#define RECORD_LEN 80
 void create_history(const char *dst, expression *e)
 {
-    int RECORD_LEN = 80;
     int WIDTH = RECORD_LEN - 12;
     struct History hist;
     char *expr = format_expression(e);