Return Styles: Pseud0ch, Terminal, Valhalla, NES, Geocities, Blue Moon. Entire thread

My first Brainfuck

Name: The Amazing Anus !Anus3nMVO2 2009-08-31 1:43

I was bored as fuck so I decided to implement a Brainfuck interpreter in C.

/* brainfuck.c ... version 0.9
 *
 * usage: brainfuck <file>
 *
 */

 
#include <stdio.h>

/* 64kb -1b each
 *
 * The Brainfuck spec says that memory should be 30000 bytes, but I prefer to
 * use a whole 64kb for both code and memory.
 *
 */
#define BRAINF_CODESIZE   65535
#define BRAINF_MEMSIZE   65535


FILE* fp;

unsigned char* code, *memory;
short unsigned int codeptr, memptr;


int main(int argc, char* argv[])
{
     if (argc != 2)
          exit(1);

     if ((fp = fopen(argv[1], "r")) == NULL)
          exit(1);

     code = malloc(BRAINF_CODESIZE);
     memory = malloc(BRAINF_MEMSIZE);

     memset(code, 0, BRAINF_CODESIZE);
     memset(memory, 0, BRAINF_MEMSIZE);

     codeptr = 0;
     memptr = 0;

     while (!feof(fp))
     {
          fread(code, BRAINF_CODESIZE -1, 1, fp);
     }


     /*** Interpreting begins ***/
     while (code[codeptr])
     {
          switch (code[codeptr])
          {
               case '>':
                    ++memptr;
                    ++codeptr;
                    break;

               case '<':
                    --memptr;
                    ++codeptr;
                    break;

               case '+':
                    ++memory[memptr];
                    ++codeptr;
                    break;

               case '-':
                    --memory[memptr];
                    ++codeptr;
                    break;

               case '.':
                    putchar(memory[memptr]);
                    ++codeptr;
                    break;

               case ',':
                    memory[memptr] = getchar();
                    ++codeptr;
                    break;

               /* :NOTE: Some kind of error handling should take
                * place if no ']' is found.
                */
               case '[':
                    if (!memory[memptr])
                    {
                         while ((code[codeptr] != ']') && (code[codeptr]))
                         {
                              ++codeptr;
                         }
                    }
                    else
                         ++codeptr;
                    break;


               /* :NOTE: Some kind of error handling should take
                * place if no '[' is found.
                */
               case ']':
                    if (memory[memptr])
                    {
                         while ((code[codeptr] != '[') && (codeptr))
                              --codeptr;
                    }
                    else
                         ++codeptr;
                    break;

               /* invalid instruction
                * For now we will just continue executing when this happens.
                * It should ignore characters 10, 13, & 32 anyways.
                */
               default:
                    ++codeptr;
                    break;
          }
     }

     free(code);
}

Name: Anonymous 2009-08-31 16:02

- Codesize and memsize should probably be 65536 instead of 65535. Remember that a size of N means that you can use elements 0 .. (N-1).
- You're relying on integer wraparound at 65536, which may or may not be correct depending on architecture. (Actually, you're relying on integer wraparound at 65535; see above.) You can use uint16_t instead if you need an unsigned integer that is always 16 bits.
- You're not checking the result of the two malloc calls. Shame on you.
- The code reading is wrong. It may read, say, 1024 bytes in the first iteration, stop on a (temporary) read error, notice that it's not yet at EOF, start again, and overwrite these 1024 bytes in the next fread call. Either stop in case of a read error and remove the loop, or handle it properly and keep tabs on the number of bytes successfully read. Also, the -1 here is wrong (the second argument to fread is a size, not a maximum value, so there's no need to subtract 1 from the real size).
- The '[' and ']' handlers are wrong, as has already been pointed out. >>5 is a good solution, but it requires you to keep a more complex state than you're currently using; if you want your implementation to be as simple and stateless as possible, you could use something like the following:


   case '[':
        if (!memory[memptr])
        {
             int bracketdepth = 1;
             while (backetdepth != 0)
             {
                 ++codeptr;
                 if (code[codeptr] == ']') bracketdepth--;
                 if (code[codeptr] == '[') bracketdepth++;
             }
             ++codeptr;
        }
        else
            ++codeptr;
        break;


It IS still a bit ugly though (although that may be more or less the point of anything brainfuck-related ;) )
- A minor stylistic issue: your code becomes somewhat clearer if you increment codeptr at the very end of the main loop, instead of at each branch. This has the downside of making [ and ] somewhat more complicated, so you have to decide for yourself whether it's actually an improvement.
- I'm pretty sure all characters other than +-<>.,[] are well-defined as being comments, so your default: clause is perfectly fine.
- Finally, you don't return a value (in particular, the value 0) at the end of main, something the compiler will probably have warned you about.

Newer Posts
Don't change these.
Name: Email:
Entire Thread Thread List