Skip to main content
1 of 13
Harith
  • 9.8k
  • 1
  • 14
  • 50

Undefined behavior:

From the C Standard: (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

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.

    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;
     }
Harith
  • 9.8k
  • 1
  • 14
  • 50