Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

minor cleanup for code scan #194

Merged
merged 15 commits into from
Sep 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,9 @@ jobs:
uses: actions/checkout@v4

- name: Run clang-format
run: ./scripts/ci-run-clang-format.sh
run: |
sudo ln -sf /usr/bin/clang-format-15 /usr/bin/clang-format
./scripts/ci-run-clang-format.sh
ci:
name: ci
Expand Down
2 changes: 1 addition & 1 deletion app/pretty.c
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,7 @@ static size_t is_newline(const unsigned char *utf8, int wchar_len) {
}

// utf8_bytes_up_to_max_width: return number of bytes used up to a maximum screen width
// max will be set to the actual width used
// set to the actual width used
static size_t utf8_bytes_up_to_max_width_and_replace_newlines(unsigned char *str1, size_t len1, size_t max_width,
size_t *used_width, int *err) {
utf8proc_int32_t codepoint1;
Expand Down
2 changes: 1 addition & 1 deletion app/rm.c
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ int ZSV_MAIN_NO_OPTIONS_FUNC(ZSV_COMMAND)(int argc, const char *argv[]) {
printf("Are you sure you want to remove the file %s%s?\n", filepath,
remove_cache ? " and all of its cache contents" : "");
char buff[64];
if (fscanf(stdin, "%60s", buff) && strchr("Yy", buff[0]))
if (fscanf(stdin, "%60s", buff) == 1 && strchr("Yy", buff[0]))
ok = 1;
}
#endif
Expand Down
27 changes: 13 additions & 14 deletions app/select-pull.c
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
#include <zsv/utils/utf8.h>
#include <zsv/utils/string.h>
#include <zsv/utils/mem.h>
#include <zsv/utils/arg.h>

struct zsv_select_search_str {
struct zsv_select_search_str *next;
Expand Down Expand Up @@ -63,7 +64,7 @@ struct zsv_select_data {
struct { // merge data: only used with --merge
struct zsv_select_uint_list *indexes, **last_index;
} merge;
} * out2in; // array of .output_cols_count length; out2in[x] = y where x = output ix, y = input info
} *out2in; // array of .output_cols_count length; out2in[x] = y where x = output ix, y = input info

unsigned int output_cols_count; // total count of output columns

Expand Down Expand Up @@ -343,23 +344,23 @@ static enum zsv_select_column_index_selection_type zsv_select_column_index_selec
unsigned *lo, unsigned *hi) {
enum zsv_select_column_index_selection_type result = zsv_select_column_index_selection_type_none;

unsigned int i, j, k;
unsigned int i = 0;
unsigned int j = 0;
int n = 0;
k = sscanf((const char *)arg, "%u-%u%n", &i, &j, &n);
int k = sscanf((const char *)arg, "%u-%u%n", &i, &j, &n);
if (k == 2) {
if (n == (int)strlen((const char *)arg) && i > 0 && j >= i)
if (n >= 0 && (size_t)n == strlen((const char *)arg) && i > 0 && j >= i)
result = zsv_select_column_index_selection_type_range;
} else {
k = sscanf((const char *)arg, "%u%n", &i, &n);
if (k && n == (int)strlen((const char *)arg)) {
if (k == 1 && n >= 0 && (size_t)n == strlen((const char *)arg)) {
if (i > 0)
result = zsv_select_column_index_selection_type_single;
} else {
k = sscanf((const char *)arg, "%u-%n", &i, &n);
if (k && n == (int)strlen((const char *)arg)) {
if (k == 1 && n >= 0 && (size_t)n == strlen((const char *)arg)) {
Copy link
Collaborator

@iamazeem iamazeem Sep 21, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was wondering if n will ever be 0 for these cases. 🤔
Checking n against strlen() seems to be enough.
Please see these examples:

n is never 0 here.

Also, the negative lower-bounded case (-5) seems to be handled as single index.
It's being read and wrapped around as its type is unsigned int.

Here's a sample CLI interaction:

Screenshot from 2024-09-21 16-16-33

if (i > 0) {
result = zsv_select_column_index_selection_type_lower_bounded;
j = 0;
}
}
}
Expand Down Expand Up @@ -616,6 +617,7 @@ int ZSV_MAIN_FUNC(ZSV_COMMAND)(int argc, const char *argv[], struct zsv_opts *op
return zsv_status_ok;
}

int err = 0;
struct zsv_select_data data = {0};
data.opts = opts;
const char *input_path = NULL;
Expand Down Expand Up @@ -708,14 +710,11 @@ int ZSV_MAIN_FUNC(ZSV_COMMAND)(int argc, const char *argv[], struct zsv_opts *op
-1, "--sample-pct value should be a number between 0 and 100 (e.g. 1.5 for a sample of 1.5%% of the data");
else
data.sample_pct = d;
} else if (!strcmp(argv[arg_i], "--prepend-header")) {
if (!(arg_i + 1 < argc))
stat = zsv_printerr(1, "%s option requires a value");
else
data.prepend_header = argv[++arg_i];
} else if (!strcmp(argv[arg_i], "--no-header")) {
} else if (!strcmp(argv[arg_i], "--prepend-header"))
data.prepend_header = zsv_next_arg(++arg_i, argc, argv, &err);
else if (!strcmp(argv[arg_i], "--no-header"))
data.no_header = 1;
} else if (!strcmp(argv[arg_i], "-H") || !strcmp(argv[arg_i], "--head")) {
else if (!strcmp(argv[arg_i], "-H") || !strcmp(argv[arg_i], "--head")) {
if (!(arg_i + 1 < argc && atoi(argv[arg_i + 1]) >= 0))
stat = zsv_printerr(1, "%s option value invalid: should be positive integer; got %s", argv[arg_i],
arg_i + 1 < argc ? argv[arg_i + 1] : "");
Expand Down
86 changes: 49 additions & 37 deletions app/select.c
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
#include <zsv/utils/utf8.h>
#include <zsv/utils/string.h>
#include <zsv/utils/mem.h>
#include <zsv/utils/arg.h>

struct zsv_select_search_str {
struct zsv_select_search_str *next;
Expand Down Expand Up @@ -69,7 +70,7 @@ struct zsv_select_data {
struct { // merge data: only used with --merge
struct zsv_select_uint_list *indexes, **last_index;
} merge;
} * out2in; // array of .output_cols_count length; out2in[x] = y where x = output ix, y = input info
} *out2in; // array of .output_cols_count length; out2in[x] = y where x = output ix, y = input info

unsigned int output_cols_count; // total count of output columns

Expand Down Expand Up @@ -353,20 +354,21 @@ static enum zsv_select_column_index_selection_type zsv_select_column_index_selec
unsigned *lo, unsigned *hi) {
enum zsv_select_column_index_selection_type result = zsv_select_column_index_selection_type_none;

unsigned int i, j, k;
unsigned int i = 0;
unsigned int j = 0;
int n = 0;
k = sscanf((const char *)arg, "%u-%u%n", &i, &j, &n);
int k = sscanf((const char *)arg, "%u-%u%n", &i, &j, &n);
if (k == 2) {
if (n == (int)strlen((const char *)arg) && i > 0 && j >= i)
if (n >= 0 && (size_t)n == strlen((const char *)arg) && i > 0 && j >= i)
result = zsv_select_column_index_selection_type_range;
} else {
k = sscanf((const char *)arg, "%u%n", &i, &n);
if (k && n == (int)strlen((const char *)arg)) {
if (k == 1 && n >= 0 && (size_t)n == strlen((const char *)arg)) {
if (i > 0)
result = zsv_select_column_index_selection_type_single;
} else {
k = sscanf((const char *)arg, "%u-%n", &i, &n);
if (k && n == (int)strlen((const char *)arg)) {
if (k == 1 && n >= 0 && (size_t)n == strlen((const char *)arg)) {
if (i > 0) {
result = zsv_select_column_index_selection_type_lower_bounded;
j = 0;
Expand Down Expand Up @@ -706,7 +708,8 @@ static enum zsv_status auto_detect_fixed_column_sizes(struct fixed *fixed, struc

// allocate offsets
free(fixed->offsets);
fixed->offsets = malloc(fixed->count * sizeof(*fixed->offsets));
fixed->offsets = NULL; // unnecessary line to silence codeQL false positive
fixed->offsets = calloc(fixed->count, sizeof(*fixed->offsets));
if (!fixed->offsets) {
stat = zsv_status_memory;
goto auto_detect_fixed_column_sizes_exit;
Expand Down Expand Up @@ -783,12 +786,17 @@ int ZSV_MAIN_FUNC(ZSV_COMMAND)(int argc, const char *argv[], struct zsv_opts *op
if (*s == ',')
data.fixed.count++;
free(data.fixed.offsets);
data.fixed.offsets = malloc(data.fixed.count * sizeof(*data.fixed.offsets));
data.fixed.offsets = NULL; // unnecessary line to silence codeQL false positive
data.fixed.offsets = calloc(data.fixed.count, sizeof(*data.fixed.offsets));
if (!data.fixed.offsets) {
stat = zsv_printerr(1, "Out of memory!\n");
break;
}
size_t count = 0;
const char *start = argv[arg_i];
for (const char *end = argv[arg_i];; end++) {
if (*end == ',' || *end == '\0') {
if (!sscanf(start, "%zu,", &data.fixed.offsets[count++])) {
if (sscanf(start, "%zu,", &data.fixed.offsets[count++]) != 1) {
Dismissed Show dismissed Hide dismissed
stat = zsv_printerr(1, "Invalid offset: %.*s\n", end - start, start);
break;
} else if (*end == '\0')
Expand Down Expand Up @@ -850,17 +858,17 @@ int ZSV_MAIN_FUNC(ZSV_COMMAND)(int argc, const char *argv[], struct zsv_opts *op
stat = zsv_printerr(1, "--sample-pct option requires a value");
else if (!(d = atof(argv[arg_i])) && d > 0 && d < 100)
stat = zsv_printerr(
-1, "--sample-pct value should be a number between 0 and 100 (e.g. 1.5 for a sample of 1.5% of the data");
-1, "--sample-pct value should be a number between 0 and 100 (e.g. 1.5 for a sample of 1.5%% of the data");
else
data.sample_pct = d;
} else if (!strcmp(argv[arg_i], "--prepend-header")) {
if (!(arg_i + 1 < argc))
stat = zsv_printerr(1, "%s option requires a value");
else
data.prepend_header = argv[++arg_i];
} else if (!strcmp(argv[arg_i], "--no-header")) {
int err = 0;
data.prepend_header = zsv_next_arg(++arg_i, argc, argv, &err);
if (err)
stat = zsv_status_error;
} else if (!strcmp(argv[arg_i], "--no-header"))
data.no_header = 1;
} else if (!strcmp(argv[arg_i], "-H") || !strcmp(argv[arg_i], "--head")) {
else if (!strcmp(argv[arg_i], "-H") || !strcmp(argv[arg_i], "--head")) {
if (!(arg_i + 1 < argc && atoi(argv[arg_i + 1]) >= 0))
stat = zsv_printerr(1, "%s option value invalid: should be positive integer; got %s", argv[arg_i],
arg_i + 1 < argc ? argv[arg_i + 1] : "");
Expand Down Expand Up @@ -898,33 +906,37 @@ int ZSV_MAIN_FUNC(ZSV_COMMAND)(int argc, const char *argv[], struct zsv_opts *op
input_path = argv[arg_i];
}

if (data.sample_pct)
srand(time(0));
if (stat == zsv_status_ok) {
if (data.sample_pct)
srand(time(0));

if (data.use_header_indexes && stat == zsv_status_ok)
stat = zsv_select_check_exclusions_are_indexes(&data);
if (data.use_header_indexes && stat == zsv_status_ok)
stat = zsv_select_check_exclusions_are_indexes(&data);
}

if (!data.opts->stream) {
if (stat == zsv_status_ok) {
if (!data.opts->stream) {
#ifdef NO_STDIN
stat = zsv_printerr(1, "Please specify an input file");
stat = zsv_printerr(1, "Please specify an input file");
#else
data.opts->stream = stdin;
data.opts->stream = stdin;
#endif
}
}

if (stat == zsv_status_ok && fixed_auto) {
if (data.fixed.offsets)
stat = zsv_printerr(zsv_status_error, "Please specify either --fixed-auto or --fixed, but not both");
else if (data.opts->insert_header_row)
stat = zsv_printerr(zsv_status_error, "--fixed-auto can not be specified together with --header-row");
else {
size_t buffsize = 1024 * 256; // read the first
preview_buff = calloc(buffsize, sizeof(*preview_buff));
if (!preview_buff)
stat = zsv_printerr(zsv_status_memory, "Out of memory!");
else
stat = auto_detect_fixed_column_sizes(&data.fixed, data.opts, preview_buff, buffsize, &preview_buff_len,
opts->verbose);
if (stat == zsv_status_ok && fixed_auto) {
if (data.fixed.offsets)
stat = zsv_printerr(zsv_status_error, "Please specify either --fixed-auto or --fixed, but not both");
else if (data.opts->insert_header_row)
stat = zsv_printerr(zsv_status_error, "--fixed-auto can not be specified together with --header-row");
else {
size_t buffsize = 1024 * 256; // read the first
preview_buff = calloc(buffsize, sizeof(*preview_buff));
if (!preview_buff)
stat = zsv_printerr(zsv_status_memory, "Out of memory!");
else
stat = auto_detect_fixed_column_sizes(&data.fixed, data.opts, preview_buff, buffsize, &preview_buff_len,
opts->verbose);
}
}
}

Expand Down
2 changes: 1 addition & 1 deletion app/sql.c
Original file line number Diff line number Diff line change
Expand Up @@ -392,7 +392,7 @@ int ZSV_MAIN_FUNC(ZSV_COMMAND)(int argc, const char *argv[], struct zsv_opts *op
for (char *ix_str = data.join_indexes; !err && ix_str && *ix_str && *(++ix_str);
ix_str = strchr(ix_str + 1, ',')) {
unsigned int next_ix;
if (sscanf(ix_str, "%u,", &next_ix)) {
if (sscanf(ix_str, "%u,", &next_ix) == 1) {
if (next_ix == 0)
fprintf(stderr, "--join-indexes index must be greater than zero\n");
else if (next_ix > (unsigned)col_count)
Expand Down
6 changes: 3 additions & 3 deletions scripts/ci-run-clang-format.sh
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@ set -e

echo "[INF] Running $0"

VERSION=$(clang-format --version | cut -d ' ' -f4 | tr -d '\n')
VERSION=$(clang-format --version | sed 's/^[^0-9]*//g' | sed 's/ .*$//g')
MAJOR_VERSION=$(echo "$VERSION" | cut -d '.' -f1)
REQUIRED_VERSION="14"
REQUIRED_VERSION="15"

if [ "$VERSION" = "" ]; then
echo "[ERR] clang-format is not installed!"
Expand All @@ -16,7 +16,7 @@ else
echo "[INF] clang-format version [$VERSION]"
if [ "$MAJOR_VERSION" -lt "$REQUIRED_VERSION" ]; then
echo "[ERR] Installed clang-format version is $VERSION."
echo "[ERR] clang-format $REQUIRED_VERSION or later is required!"
echo "[ERR] clang-format-$REQUIRED_VERSION or later is required!"
exit 1
fi
fi
Expand Down
2 changes: 1 addition & 1 deletion src/zsv_internal.c
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ struct zsv_scanner {
union {
struct zsv_scan_delim_regs delim;
struct zsv_scan_fixed_regs fixed;
} * regs;
} *regs;
enum zsv_status stat; // last status
unsigned char *buff;
size_t bytes_read;
Expand Down