From eece8c85564b9c5ae0cb3edfe7edb684242e9227 Mon Sep 17 00:00:00 2001 From: Bruce Hill Date: Mon, 18 Jan 2021 10:30:17 -0800 Subject: [PATCH] More static analyzer cleanup, including switching to use bools where appropriate and EXIT_SUCESS/EXIT_FAILURE --- Makefile | 2 +- bp.c | 94 +++++++++++++++++++++++++++++-------------------------- files.c | 6 ++-- files.h | 2 +- json.c | 8 ++--- json.h | 6 +++- match.c | 11 ++++--- match.h | 3 +- pattern.h | 2 +- types.h | 8 ++--- utils.c | 12 +++---- utils.h | 8 +++-- 12 files changed, 88 insertions(+), 74 deletions(-) diff --git a/Makefile b/Makefile index 73eba4f..87ff4b0 100644 --- a/Makefile +++ b/Makefile @@ -28,7 +28,7 @@ leaktest: valgrind --leak-check=full ./bp -l -g grammars/bp.bp -p Grammar grammars/bp.bp splint: - splint +posixlib $(CFILES) bp.c + splint -posix-lib $(CFILES) bp.c install: $(NAME) mkdir -p -m 755 "$(PREFIX)/share/man/man1" "$(PREFIX)/bin" "$(SYSCONFDIR)/xdg/$(NAME)" diff --git a/bp.c b/bp.c index 5d76dd8..46b5c2b 100644 --- a/bp.c +++ b/bp.c @@ -8,6 +8,7 @@ #include #include #include +#include #include #include #include @@ -50,10 +51,11 @@ static const char *usage = ( #define USE_DEFAULT_CONTEXT -2 #define ALL_CONTEXT -1 static int context_lines = USE_DEFAULT_CONTEXT; -static unsigned int print_color = 0; -static unsigned int print_line_numbers = 0; -static unsigned int ignorecase = 0; -static unsigned int verbose = 0; +static bool print_color = false; +static bool print_line_numbers = false; +static bool ignorecase = false; +static bool verbose = false; +static bool git_mode = false; typedef enum { CONFIRM_ASK, CONFIRM_ALL, CONFIRM_NONE } confirm_t; static confirm_t confirm = CONFIRM_ALL; static enum { @@ -195,7 +197,7 @@ static void cleanup(void) static void sig_handler(int sig) { cleanup(); - if (kill(0, sig)) _exit(1); + if (kill(0, sig)) _exit(EXIT_FAILURE); } // @@ -276,7 +278,7 @@ static int inplace_modify_file(def_t *defs, file_t *f, pat_t *pattern) ++matches; printer_t err_pr = {.file = f, .context_lines = 1, .use_color = 1, .print_line_numbers = 1}; if (print_errors(&err_pr, m) > 0) - exit(1); + exit(EXIT_FAILURE); // Lazy-open file for writing upon first match: if (inplace_file == NULL) { check(snprintf(tmp_filename, PATH_MAX, "%s.tmp.XXXXXX", f->filename) <= PATH_MAX, @@ -328,7 +330,7 @@ static int print_matches(def_t *defs, file_t *f, pat_t *pattern) for (match_t *m = NULL; (m = next_match(defs, f, m, pattern, ignorecase)); ) { printer_t err_pr = {.file = f, .context_lines = 1, .use_color = 1, .print_line_numbers = 1}; if (print_errors(&err_pr, m) > 0) - exit(1); + exit(EXIT_FAILURE); if (++matches == 1) { if (printed_filenames++ > 0) printf("\n"); @@ -415,8 +417,8 @@ static int process_dir(def_t *defs, const char *dirname, pat_t *pattern) return matches; } -#define FLAG(f) (flag=getflag((f), argv, &i)) -#define BOOLFLAG(f) (boolflag((f), argv, &i)) +#define FLAG(f) (flag=getflag((f), argv, &argi)) +#define BOOLFLAG(f) (boolflag((f), argv, &argi)) int main(int argc, char *argv[]) { @@ -432,16 +434,16 @@ int main(int argc, char *argv[]) file_t *local_file = load_filef(&loaded_files, "%s/.config/"BP_NAME"/builtins.bp", getenv("HOME")); if (local_file) defs = load_grammar(defs, local_file); - int i, git = 0; - for (i = 1; i < argc; i++) { - if (streq(argv[i], "--")) { - ++i; + int argi; + for (argi = 1; argi < argc; argi++) { + if (streq(argv[argi], "--")) { + ++argi; break; } else if (BOOLFLAG("-h") || BOOLFLAG("--help")) { printf("%s\n", usage); return 0; } else if (BOOLFLAG("-v") || BOOLFLAG("--verbose")) { - verbose = 1; + verbose = true; } else if (BOOLFLAG("-e") || BOOLFLAG("--explain")) { mode = MODE_EXPLAIN; } else if (BOOLFLAG("-j") || BOOLFLAG("--json")) { @@ -451,9 +453,9 @@ int main(int argc, char *argv[]) } else if (BOOLFLAG("-C") || BOOLFLAG("--confirm")) { confirm = CONFIRM_ASK; } else if (BOOLFLAG("-G") || BOOLFLAG("--git")) { - git = 1; + git_mode = true; } else if (BOOLFLAG("-i") || BOOLFLAG("--ignore-case")) { - ignorecase = 1; + ignorecase = true; } else if (BOOLFLAG("-l") || BOOLFLAG("--list-files")) { mode = MODE_LISTFILES; } else if (FLAG("-r") || FLAG("--replace")) { @@ -498,17 +500,17 @@ int main(int argc, char *argv[]) context_lines = 0; else context_lines = (int)strtol(flag, NULL, 10); - } else if (argv[i][0] == '-' && argv[i][1] && argv[i][1] != '-') { // single-char flags - printf("Unrecognized flag: -%c\n\n%s\n", argv[i][1], usage); + } else if (argv[argi][0] == '-' && argv[argi][1] && argv[argi][1] != '-') { // single-char flags + printf("Unrecognized flag: -%c\n\n%s\n", argv[argi][1], usage); return 1; - } else if (argv[i][0] != '-') { + } else if (argv[argi][0] != '-') { if (pattern != NULL) break; - file_t *arg_file = spoof_file(&loaded_files, "", argv[i]); + file_t *arg_file = spoof_file(&loaded_files, "", argv[argi]); pat_t *p = bp_stringpattern(arg_file, arg_file->contents); - check(p, "Pattern failed to compile: %s", argv[i]); + check(p, "Pattern failed to compile: %s", argv[argi]); pattern = chain_together(arg_file, pattern, p); } else { - printf("Unrecognized flag: %s\n\n%s\n", argv[i], usage); + printf("Unrecognized flag: %s\n\n%s\n", argv[argi], usage); return 1; } } @@ -517,8 +519,8 @@ int main(int argc, char *argv[]) if (context_lines < 0 && context_lines != ALL_CONTEXT) context_lines = 0; if (isatty(STDOUT_FILENO)) { - print_color = 1; - print_line_numbers = 1; + print_color = true; + print_line_numbers = true; } // If any of these signals triggers, and there is a temporary file in use, @@ -526,10 +528,10 @@ int main(int argc, char *argv[]) int signals[] = {SIGTERM, SIGINT, SIGXCPU, SIGXFSZ, SIGVTALRM, SIGPROF, SIGSEGV, SIGTSTP}; struct sigaction sa = {.sa_handler = &sig_handler, .sa_flags = (int)(SA_NODEFER | SA_RESETHAND)}; for (size_t i = 0; i < sizeof(signals)/sizeof(signals[0]); i++) - check(!sigaction(signals[i], &sa, NULL), "Failed to set signal handler"); + check(sigaction(signals[i], &sa, NULL) == 0, "Failed to set signal handler"); // Handle exit() calls gracefully: - check(!atexit(&cleanup), "Failed to set cleanup handler at exit"); + check(atexit(&cleanup) == 0, "Failed to set cleanup handler at exit"); // User input/output is handled through /dev/tty so that normal unix pipes // can work properly while simultaneously asking for user input. @@ -564,32 +566,36 @@ int main(int argc, char *argv[]) free(patstr); } + check(pattern != NULL, "No pattern was given"); + // To ensure recursion (and left recursion in particular) works properly, // we need to define a rule called "pattern" with the value of whatever // pattern the args specified, and use `pattern` as the thing being matched. - defs = with_def(defs, strlen("pattern"), "pattern", pattern); // TODO: this is a bit hacky - pattern = bp_pattern(loaded_files, "pattern"); + defs = with_def(defs, strlen("pattern"), "pattern", pattern); + file_t *patref_file = spoof_file(&loaded_files, "", "pattern"); + pattern = bp_pattern(patref_file, patref_file->contents); int found = 0; if (mode == MODE_JSON) printf("["); - if (git) { // Get the list of files from `git --ls-files ...` + if (git_mode) { // Get the list of files from `git --ls-files ...` int fds[2]; check(pipe(fds) == 0, "Failed to create pipe"); pid_t child = fork(); check(child != -1, "Failed to fork"); if (child == 0) { - char **git_args = calloc((size_t)(2+(argc-i)+1), sizeof(char*)); + char **git_args = memcheck(calloc((size_t)(2+(argc-argi)+1), sizeof(char*))); int g = 0; git_args[g++] = "git"; git_args[g++] = "ls-files"; - while (i < argc) git_args[g++] = argv[i++]; - check(dup2(fds[STDOUT_FILENO], STDOUT_FILENO), "Failed to hook up pipe to stdout"); - check(!close(fds[STDIN_FILENO]), "Failed to close read end of pipe"); + while (argi < argc) git_args[g++] = argv[argi++]; + check(dup2(fds[STDOUT_FILENO], STDOUT_FILENO) == STDOUT_FILENO, + "Failed to hook up pipe to stdout"); + check(close(fds[STDIN_FILENO]) == 0, "Failed to close read end of pipe"); (void)execvp("git", git_args); - _exit(1); + _exit(EXIT_FAILURE); } - check(!close(fds[STDOUT_FILENO]), "Failed to close write end of pipe"); - char path[PATH_MAX+2] = {0}; // path + \n + \0 + check(close(fds[STDOUT_FILENO]) == 0, "Failed to close write end of pipe"); + char path[PATH_MAX+2] = {'\0'}; // path + \n + \0 while (read(fds[STDIN_FILENO], path, PATH_MAX+1) > 0) { // Iterate over chunks for (char *nl; (nl = strchr(path, '\n')); ) { // Iterate over nl-terminated lines *nl = '\0'; @@ -597,19 +603,19 @@ int main(int argc, char *argv[]) memmove(path, nl+1, sizeof(path)-(size_t)(nl+1-path)); } } - check(!close(fds[STDIN_FILENO]), "Failed to close read end of pipe"); + check(close(fds[STDIN_FILENO]) == 0, "Failed to close read end of pipe"); int status; while (waitpid(child, &status, 0) != child) continue; - check(WIFEXITED(status) && WEXITSTATUS(status) == 0, + check((WIFEXITED(status) == 1) && (WEXITSTATUS(status) == 0), "`git --ls-files` failed. Do you have git installed?"); - } else if (i < argc) { + } else if (argi < argc) { // Files pass in as command line args: struct stat statbuf; - for (int nfiles = 0; i < argc; nfiles++, i++) { - if (stat(argv[i], &statbuf) == 0 && S_ISDIR(statbuf.st_mode)) // Symlinks are okay if manually specified - found += process_dir(defs, argv[i], pattern); + for (int nfiles = 0; argi < argc; nfiles++, argi++) { + if (stat(argv[argi], &statbuf) == 0 && S_ISDIR(statbuf.st_mode)) // Symlinks are okay if manually specified + found += process_dir(defs, argv[argi], pattern); else - found += process_file(defs, argv[i], pattern); + found += process_file(defs, argv[argi], pattern); } } else if (isatty(STDIN_FILENO)) { // No files, no piped in input, so use files in current dir, recursively @@ -636,7 +642,7 @@ int main(int argc, char *argv[]) free_all_matches(); #endif - return (found > 0) ? 0 : 1; + return (found > 0) ? EXIT_SUCCESS : EXIT_FAILURE; } // vim: ts=4 sw=0 et cino=L2,l1,(0,W4,m1 diff --git a/files.c b/files.c index 170c8ce..d111e4f 100644 --- a/files.c +++ b/files.c @@ -93,7 +93,7 @@ file_t *load_file(file_t **files, const char *filename) } finished_loading: - if (fd != STDIN_FILENO) check(!close(fd), "Failed to close file"); + if (fd != STDIN_FILENO) check(close(fd) == 0, "Failed to close file"); f->end = &f->contents[length]; populate_lines(f); if (files != NULL) { @@ -131,7 +131,7 @@ void intern_file(file_t *f) size_t size = (size_t)(f->end - f->contents); char *buf = xcalloc(sizeof(char), size + 1); memcpy(buf, f->contents, size); - check(!munmap(f->contents, size), + check(munmap(f->contents, size) == 0, "Failure to un-memory-map some memory"); f->contents = buf; f->end = buf + size; @@ -156,7 +156,7 @@ void destroy_file(file_t **f) if ((*f)->contents) { if ((*f)->mmapped) { - check(!munmap((*f)->contents, (size_t)((*f)->end - (*f)->contents)), + check(munmap((*f)->contents, (size_t)((*f)->end - (*f)->contents)) == 0, "Failure to un-memory-map some memory"); (*f)->contents = NULL; } else { diff --git a/files.h b/files.h index a739d00..b78e77e 100644 --- a/files.h +++ b/files.h @@ -13,7 +13,7 @@ typedef struct file_s { const char *filename; char *contents, **lines, *end; size_t nlines; - /*@only@*/ struct allocated_pat_s *pats; + struct allocated_pat_s *pats; unsigned int mmapped:1; } file_t; diff --git a/json.c b/json.c index 6ec9fcd..18b1fa6 100644 --- a/json.c +++ b/json.c @@ -4,17 +4,17 @@ #include -#include "types.h" +#include "json.h" __attribute__((nonnull)) -static int _json_match(const char *text, match_t *m, int comma, unsigned int verbose); +static int _json_match(const char *text, match_t *m, int comma, bool verbose); // // Helper function for json_match(). // `comma` is used to track whether a comma will need to be printed before the // next object or not. // -static int _json_match(const char *text, match_t *m, int comma, unsigned int verbose) +static int _json_match(const char *text, match_t *m, int comma, bool verbose) { if (!verbose) { if (m->pat->type != BP_REF) { @@ -49,7 +49,7 @@ static int _json_match(const char *text, match_t *m, int comma, unsigned int ver // // Print a match object as a JSON object. // -void json_match(const char *text, match_t *m, unsigned int verbose) +void json_match(const char *text, match_t *m, bool verbose) { (void)_json_match(text, m, 0, verbose); } diff --git a/json.h b/json.h index 65fa718..554c88b 100644 --- a/json.h +++ b/json.h @@ -4,8 +4,12 @@ #ifndef JSON__H #define JSON__H +#include + +#include "types.h" + __attribute__((nonnull)) -void json_match(const char *text, match_t *m, unsigned int verbose); +void json_match(const char *text, match_t *m, bool verbose); #endif // vim: ts=4 sw=0 et cino=L2,l1,(0,W4,m1 diff --git a/match.c b/match.c index e771cce..b30a183 100644 --- a/match.c +++ b/match.c @@ -33,17 +33,18 @@ static match_t *unused_matches = NULL; static match_t *in_use_matches = NULL; #endif +__attribute__((returns_nonnull)) static match_t *new_match(void); __attribute__((nonnull, pure)) static inline const char *next_char(file_t *f, const char *str); __attribute__((nonnull)) -static const char *match_backref(const char *str, match_t *cap, unsigned int ignorecase); +static const char *match_backref(const char *str, match_t *cap, bool ignorecase); __attribute__((nonnull)) static match_t *get_capture_by_num(match_t *m, int *n); __attribute__((nonnull, pure)) static match_t *get_capture_by_name(match_t *m, const char *name); __attribute__((hot, nonnull(2,3,4))) -static match_t *match(def_t *defs, file_t *f, const char *str, pat_t *pat, unsigned int flags); +static match_t *match(def_t *defs, file_t *f, const char *str, pat_t *pat, bool ignorecase); // // Return the location of the next character or UTF8 codepoint. @@ -69,7 +70,7 @@ static inline const char *next_char(file_t *f, const char *str) // Attempt to match text against a previously captured value. // Return the character position after the backref has matched, or NULL if no match has occurred. // -static const char *match_backref(const char *str, match_t *cap, unsigned int ignorecase) +static const char *match_backref(const char *str, match_t *cap, bool ignorecase) { if (cap->pat->type == BP_REPLACE) { const char *text = cap->pat->args.replace.text; @@ -126,7 +127,7 @@ static const char *match_backref(const char *str, match_t *cap, unsigned int ign // // Find the next match after prev (or the first match if prev is NULL) // -match_t *next_match(def_t *defs, file_t *f, match_t *prev, pat_t *pat, unsigned int ignorecase) +match_t *next_match(def_t *defs, file_t *f, match_t *prev, pat_t *pat, bool ignorecase) { const char *str; if (prev) { @@ -147,7 +148,7 @@ match_t *next_match(def_t *defs, file_t *f, match_t *prev, pat_t *pat, unsigned // match object, or NULL if no match is found. // The returned value should be free()'d to avoid memory leaking. // -static match_t *match(def_t *defs, file_t *f, const char *str, pat_t *pat, unsigned int ignorecase) +static match_t *match(def_t *defs, file_t *f, const char *str, pat_t *pat, bool ignorecase) { switch (pat->type) { case BP_LEFTRECURSION: { diff --git a/match.h b/match.h index 3386a70..ee6fe6a 100644 --- a/match.h +++ b/match.h @@ -4,12 +4,13 @@ #ifndef MATCH__H #define MATCH__H +#include #include #include "types.h" __attribute__((nonnull(2,4))) -match_t *next_match(def_t *defs, file_t *f, match_t *prev, pat_t *pat, unsigned int flags); +match_t *next_match(def_t *defs, file_t *f, match_t *prev, pat_t *pat, bool ignorecase); __attribute__((nonnull)) match_t *get_capture(match_t *m, const char **id); __attribute__((nonnull)) diff --git a/pattern.h b/pattern.h index ff1105c..497fe76 100644 --- a/pattern.h +++ b/pattern.h @@ -7,7 +7,7 @@ #include "files.h" #include "types.h" -__attribute__((nonnull)) +__attribute__((returns_nonnull, nonnull)) pat_t *new_pat(file_t *f, const char *start, enum pattype_e type); __attribute__((nonnull(1,2))) pat_t *bp_stringpattern(file_t *f, const char *str); diff --git a/types.h b/types.h index 037e805..28b0d07 100644 --- a/types.h +++ b/types.h @@ -81,7 +81,7 @@ typedef struct pat_s { // // Pattern matching result object // -typedef struct match_s { +typedef /*@refcounted@*/ struct match_s { // Where the match starts and ends (end is after the last character) const char *start, *end; struct match_s *child, *nextsibling; @@ -91,7 +91,7 @@ typedef struct match_s { #ifdef DEBUG_HEAP struct match_s **atme; #endif - int refcount; + /*@refs@*/ int refcount; // If skip_replacement is set to 1, that means the user wants to not print // the replaced text when printing this match: // TODO: this is a bit hacky, there is probably a better way to go about @@ -106,7 +106,7 @@ typedef struct def_s { size_t namelen; const char *name; pat_t *pat; - /*@only@*/ struct def_s *next; + struct def_s *next; } def_t; // @@ -114,7 +114,7 @@ typedef struct def_s { // file is freed. // typedef struct allocated_pat_s { - /*@only@*/ struct allocated_pat_s *next; + struct allocated_pat_s *next; pat_t pat; } allocated_pat_t; diff --git a/utils.c b/utils.c index 76651bc..58f8e2d 100644 --- a/utils.c +++ b/utils.c @@ -50,27 +50,27 @@ const char *after_name(const char *str) // // Check if a character is found and if so, move past it. // -int matchchar(const char **str, char c) +bool matchchar(const char **str, char c) { const char *next = after_spaces(*str); if (*next == c) { *str = &next[1]; - return 1; + return true; } - return 0; + return false; } // // Check if a string is found and if so, move past it. // -int matchstr(const char **str, const char *target) +bool matchstr(const char **str, const char *target) { const char *next = after_spaces(*str); if (strncmp(next, target, strlen(target)) == 0) { *str = &next[strlen(target)]; - return 1; + return true; } - return 0; + return false; } // diff --git a/utils.h b/utils.h index 8080ee5..f3f81f0 100644 --- a/utils.h +++ b/utils.h @@ -4,14 +4,16 @@ #ifndef UTILS__H #define UTILS__H +#include #include +#include #include #include #include "match.h" #define streq(a, b) (strcmp(a, b) == 0) -#define check(cond, ...) do { if (!(cond)) { (void)fprintf(stderr, __VA_ARGS__); (void)fwrite("\n", 1, 1, stderr); exit(1); } } while(0) +#define check(cond, ...) do { if (!(cond)) { (void)fprintf(stderr, __VA_ARGS__); (void)fwrite("\n", 1, 1, stderr); exit(EXIT_FAILURE); } } while(0) #define new(t) memcheck(calloc(1, sizeof(t))) #define xcalloc(a,b) memcheck(calloc(a,b)) #define xrealloc(a,b) memcheck(realloc(a,b)) @@ -23,9 +25,9 @@ const char *after_name(const char *str); __attribute__((pure, nonnull, returns_nonnull)) const char *after_spaces(const char *str); __attribute__((nonnull)) -int matchchar(const char **str, char c); +bool matchchar(const char **str, char c); __attribute__((nonnull)) -int matchstr(const char **str, const char *target); +bool matchstr(const char **str, const char *target); __attribute__((nonnull)) size_t unescape_string(char *dest, const char *src, size_t bufsize); __attribute__((returns_nonnull))