**Enable more compiler warnings:** ``` gcc -std=c99 -fPIC -gdwarf-4 -Wall -Wextra -Wwrite-strings -Wno-parentheses -Wpedantic -Warray-bounds -Wconversion -Wstrict-prototypes -fanalyzer brainfuck.c -o brainfuck brainfuck.c:31:1: warning: type qualifiers ignored on function return type [-Wignored-qualifiers] 31 | const SourceFile readSourceFile(const char* filePath) { | ^~~~~ brainfuck.c: In function ‘readSourceFile’: brainfuck.c:44:13: warning: conversion to ‘size_t’ {aka ‘long unsigned int’} from ‘long int’ may change the sign of the result [-Wsign-conversion] 44 | lSize = ftell(fp); | ^~~~~ brainfuck.c: In function ‘checkSourceFileValidity’: brainfuck.c:81:26: warning: conversion to ‘uint8_t’ {aka ‘unsigned char’} from ‘char’ may change the sign of the result [-Wsign-conversion] 81 | uint8_t symbol = sourceFile->contents[i]; | ^~~~~~~~~~ brainfuck.c: In function ‘main’: brainfuck.c:153:26: warning: conversion to ‘uint8_t’ {aka ‘unsigned char’} from ‘char’ may change the sign of the result [-Wsign-conversion] 153 | uint8_t symbol = sourceFile.contents[i]; | ^~~~~~~~~~ brainfuck.c:172:38: warning: conversion from ‘int’ to ‘uint8_t’ {aka ‘unsigned char’} may change value [-Wconversion] 172 | tape[tapePosition] = getchar(); | ^~~~~~~ brainfuck.c:134:5: warning: use of possibly-NULL ‘tape’ where non-null expected [CWE-690] [-Wanalyzer-possible-null-argument] 134 | memset(tape, 0, tapeSize * sizeof(uint8_t)); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ‘main’: events 1-4 | | 108 | if (argc < 2) { | | ^ | | | | | (1) following ‘false’ branch (when ‘argc > 1’)... |...... | 118 | if (argc == 3) { | | ~ | | | | | (2) ...to here |...... | 133 | uint8_t* tape = (uint8_t*)malloc(tapeSize * sizeof(uint8_t)); | | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ | | | | | (3) this call could return NULL | 134 | memset(tape, 0, tapeSize * sizeof(uint8_t)); | | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ | | | | | (4) argument 1 (‘tape’) from (3) could be NULL where non-null expected | In file included from brainfuck.c:7: /usr/include/string.h:61:14: note: argument 1 of ‘memset’ must be non-null 61 | extern void *memset (void *__s, int __c, size_t __n) __THROW __nonnull ((1)); | ^~~~~~ ``` These are all avoidable. **Undefined behavior:** From the [C Standard](http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1256.pdf): (footnote 234 on p. 267 of the linked Standard) > Setting the file position indicator to end-of-file, as with fseek(file, > 0, SEEK_END),has undefined behavior for a binary stream (because of > possible trailing null characters) or for anystream with > state-dependent encoding that does not assuredly end in the initial > shift state. The call to `fseek()` in `readSourceFile()` invokes undefined behavior. **Declare variables where they are needed**: Starting from C99, variables do not have to be declared at the beginning of a function (or block); instead, you can declare them at the point where they are needed within the function's body. **fopen() is not required to set `errno`:** The error-handling is inconsistent. At times, we are using `perror()`, and at times we're calling fputs() with an error message. Consider: ``` #if 0 fp = fopen (filePath , "rb"); if(!fp) { perror(filePath); exit(IO_ERR); } #else errno = 0; fp = fopen(filePath, "rb"); /* The cast void is required as ISO C forbids conditional expr with only one * void side. */ if (!fp) { errno ? perror(filePath) : (void) fputs("Error - failed to open file.", stderr); exit(IO_ERR); } #endif ``` **Noise:** `const` serves no purpose with `const SourceFile readSourceFile` in the declaration `const SourceFile readSourceFile(const char* filePath)`. It is ignored. **Send error messages to `stderr`:** ``` if (argc < 2) { // puts("Error! Expected path to source file as command line argument."); fputs("Error! Expected path to source file as command line argument.\n", stderr); return USAGE_ERR; } ``` Note that `fputs()` doesn't automatically append a newline like `puts()`. **Minor:** ``` /* if we have 3 arguments, 3rd argument will be the tape size in bytes we will try and parse it as a size_t */ ``` Does it matter if there are more than 3 arguments? **Use more `const`:** In `main()`: ``` // char* str = argv[2]; const char *str = argv[2]; ``` **Ignoring the return of `malloc()` and family risks invoking undefined behavior:** ``` // Initial tape of memory and zero it all uint8_t* tape = (uint8_t*)malloc(tapeSize * sizeof(uint8_t)); memset(tape, 0, tapeSize * sizeof(uint8_t)); ``` `malloc()` and family returns `NULL` to indicate failure. The returned `void *` need not be casted as there is an implicit conversion to and from a `void *` to any other pointer type in C. ``` /* Allocate to the referenced object, not the type. * It is easier to maintain. */ uint8_t tape = malloc(tapesize * sizeof *tape); if (!tape) { complain(); .. } ``` As suggested in the comments, `malloc()` + `memset()` can be replaced with `calloc()`. **`getchar()` returns an `int`, not a `uint8_t`:** ``` // ?? case ',': { tape[tapePosition] = getchar(); break; } ``` We are missing a `\n` in all the calls to `fprintf()` and `fputs()`.