From e6a37711d2d61957d2d9c83acfe2754ed313adc1 Mon Sep 17 00:00:00 2001 From: Nicklas Larsson Date: Mon, 8 Jan 2024 21:05:35 +0100 Subject: [PATCH] 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 | 2 +- .github/workflows/ubuntu.yml | 2 +- imagery/i.landsat.acca/algorithm.c | 8 +++- lib/gis/strings.c | 17 +++---- lib/imagery/iscatt_core.c | 75 ++++++++++++++++-------------- raster/r.fill.stats/main.c | 15 +++--- raster/r.mapcalc/map.c | 2 +- 7 files changed, 67 insertions(+), 54 deletions(-) diff --git a/.github/workflows/macos_install.sh b/.github/workflows/macos_install.sh index 79828b933f6..e6b36e64680 100755 --- a/.github/workflows/macos_install.sh +++ b/.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" diff --git a/.github/workflows/ubuntu.yml b/.github/workflows/ubuntu.yml index 032234db885..194ca8dafa7 100644 --- a/.github/workflows/ubuntu.yml +++ b/.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 diff --git a/imagery/i.landsat.acca/algorithm.c b/imagery/i.landsat.acca/algorithm.c index 9987e2782a7..a77e8c0513d 100644 --- a/imagery/i.landsat.acca/algorithm.c +++ b/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; } diff --git a/lib/gis/strings.c b/lib/gis/strings.c index 94b71877818..a0a31821555 100644 --- a/lib/gis/strings.c +++ b/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; } /*! diff --git a/lib/imagery/iscatt_core.c b/lib/imagery/iscatt_core.c index 7b0ee80169b..05ce619eef8 100644 --- a/lib/imagery/iscatt_core.c +++ b/lib/imagery/iscatt_core.c @@ -628,13 +628,11 @@ 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); @@ -642,15 +640,20 @@ static void free_compute_scatts_data(int *fd_bands, struct rast_row *bands_rows, } } - 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; } diff --git a/raster/r.fill.stats/main.c b/raster/r.fill.stats/main.c index 5a99dd19092..df256ee5266 100644 --- a/raster/r.fill.stats/main.c +++ b/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 { diff --git a/raster/r.mapcalc/map.c b/raster/r.mapcalc/map.c index 9ea0fafd439..118453c0610 100644 --- a/raster/r.mapcalc/map.c +++ b/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);