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;
}