Basic Brainfuck Interpreter in C

I have written a small Brainfuck interpreter in C. More recent version hosted here.

I have found it very hard to test my interpreter because writing a program in Brainfuck is somewhat hard (or at least very tedious) and many snippets I found online either don't give output or do so very cryptically, so I don't know 100% sure if everything works as intended.

I have tested a few programs on it though, and that which I tested works.

Any criticism/advice would be very nice.

``
#include

#define MEMSIZE 30000

int memory[MEMSIZE];
int instructions[MEMSIZE];

int memptr = 0;
int insptr = 0;

int isInstruction(char);
void runInstruction(char);

int main(int argc, char *argv[]) {
// Open input file
FILE *file = fopen(argv[1], "r");
if (file == 0){
printf("Error: Could not open source file\n");
return 0;
}

// Read all instructions from input
char c;
int i = 0;
int balance = 0; // Keeps track of [ and ] balance
while ((c = fgetc(file)) != EOF && i = MEMSIZE){
printf("Error: cannot read more than 30000 instructions\n");
return 0;
}

// Error if [ and ] aren't balanced
if (balance != 0){
printf("Error: [ and ] were not balanced\n");
return 0;
}

// Run the program
while(insptr ' || c == '':
if (memptr 0)
memptr--;
else
memptr = MEMSIZE - 1;
break;
case '[':
if (memory[memptr] == 0)
// seek ] forward
while(instructions[insptr] != ']'){
insptr++;
}
break;
case ']':
if (memory[memptr] != 0)
// seek [ backward
while(instructions[insptr] != '['){
insptr--;
}
break;
case '.':
putchar(memory[memptr]);
break;
case ',':
memory[memptr] = getchar();
break;
default:
printf("Error: Encountered unknown instruction %c`\n",

Use NULL for null pointers

NULL conveys your intention to use a pointer better than 0:

if (file == NULL) { ... }

As a bonus, in C the macro NULL typically expands to something like (void*)0; if you have warnings for potentially unsafe type conversions enabled (-Wconversion for GCC), the compiler will notify you, if you try to assign 0 to a pointer or NULL to an integer, which helps to find potential errors.

Use puts when you don't use string formatting

printf("Error: Could not open source file\n");

could just as well be

puts("Error: Could not open source file");

While printf is great for formatted output, the look for formatting specifiers carries a computational overhead that puts doesn't have. Some compilers (like GCC) will perform that optimization automatically for you, if you use a format string literal without format specifiers.

Checking whether a characters is in a set of characters

In isInstruction(char), instead of using a series of disjoint (joint with the || operator) equality comparisons, you could much better express your intent and the source character set with strchr:

return strchr("+-><[].,", c) != NULL;

A possible improvements might be:

const static char brainfuck_alphabet[] = {'+', '-', '>', '<', '[', ']', '.', ','};
return memchr(brainfuck_alphabet, c, sizeof(brainfuck_alphabet));

The improvements lies in the fact that the algorithm of memchr can make an assumption about the size of the memory region to check while strchr cannot. This could lead to a performance improvement.

Use bool for boolean values

The header ` declares boolean-like types. You can use them to express your intention for a boolean return value of isInstruction():

bool isInstruction(char c) { ... }

Don't repeat yourself

While you generally did will to adhere to that principle, I want to point out one small spot that could use improvement:

printf("Error: cannot read more than 30000 instructions\n");

could be

printf("Error: cannot read more than %d instructions\n", MEMSIZE);

That way the error message would reflect the actual buffer size in case MEMSIZE was redefined. Since the argument list of printf is variadic and doesn't carry type information, it would be better yet to not make assumptions on the width of the integer type of MEMSIZE. What if someone defined MEMSIZE to be larger than MAX_INT? The best course of action would be to force MEMSIZE into the type size_t (more about that later).

printf("Error: cannot read more than %zu instructions\n", (size_t)(MEMSIZE));

Data types

-
You only ever store chars in instructions, so its type could just as well be:

char instructions[…];

On all 32- and 64-bit platforms you just saved 3*MEMSIZE bytes (which you could use to increase MEMSIZE again if that is a concern).

-
memptr, insptr and i are indexes of plain arrays. Therefore they should have type size_t, which is the type meant to reflect the size or position of things in memory.

Make the code flow of branches based on mutually exclusive conditions mutually exclusive

In the above code the conditions c == '[' and c == ']' cannot both be true since c isn't altered between them. To make this obvious to the reader and the compiler (though a good one should find that optimization itself), you should use mutually exclusive if … else if … statements:

if (c == '[')
            balance++;
        else if (c == ']')
            balance--;

Exit with an error status in case of an error

Programs may convey information in their exit status. By convention the status 0 means “no error, everything performed according to your instructions”. Every other status value usually refers to an error condition but there is no general convention on what each value means. One attempt to conventionalize them can be found in the sysexits.h header. You could use it like this in your program:

if (file == NULL) {
    // ...
    return EX_NOINPUT;
}

if (balance = MEMSIZE) {
    // ...
    return EX_SOFTWARE;
}

In a similar fashion you could return an error from runInstruction(), but we already know that we didn't accept any illegal instructions.

Print error messages on stderr

printf and puts write their data to stdout, the standard output stream, but you're also writing program output to stdout via putchar. To avoid mixing program output and error messages, you should write the latter to stderr instead:

fprintf(stderr, ...);
fputs(..., stderr);

Be aware that fputs doesn't append a new-line character like puts.

Print operating system error messages to provide hints to the user

Like many library functions relying on system calls, fopen sets the global errno variable to report the type of error. You can use functions like strerror or perror to decode the value of errno` into a human-readable, localized(!) error me

StackExchange Code Review Q#134489, answer score: 12