BrainFK -> C converter

Question

This program converts .b brainfk source code to C, and compiles with gcc. It runs very well (that was my first time I played Lost Kingdom), however, the code is quite long because some parts are repeated.

Is there a way to reduce the code? What about the "optimization" method? Can this simple optimization can be improved?

PS : This code is for Linux, but can be changed for Windows.

PPS : I'm bad at manipulating string in C...

#include<stdio.h>
#include<stdbool.h>
#include<string.h>
#include<stdlib.h>

int main(int argv, char* argc[]){
    //help message
    if(argv==1){
        printf("\n");
        printf("How to use : %s filename [-o output|-c|-d|-r]\n",argc[0]);
        printf("-o output : Set output file name\n-c : Do not compile\n-d : Do not delete C source file\n");
        printf("INFO : You SHOULD type 'export MALLOC_CHECK_=0' manually to remove warning.\n");
        printf("\n");
        return 0;
    }
    printf("INFO : You may type 'export MALLOC_CHECK_=0' manually to remove warning.\n");
    int i;
    bool doCompile = true;
    bool deleteSource = true;
    char* fileName = argc[1];
    char* outFileName;
    outFileName = (char*)malloc(1024);
    strncpy(outFileName,fileName,1000);
    strcat(outFileName,".o");
    bool isSetOut = false;
    //set flags 
    for(i=2;i<argv;i++){
        if(isSetOut){
            isSetOut = false;
            outFileName = argc[i];
        }
        else if(strcmp(argc[i],"-c")==0){
            doCompile = false;
        }
        else if(strcmp(argc[i],"-d")==0){
            deleteSource = false;
        }
        else if(strcmp(argc[i],"-o")==0){
            isSetOut = true;
        }
    }
    char outFileC[1024];
    strncpy(outFileC,outFileName,1000);
    strcat(outFileC,".c");

    //bf file
    FILE* bfFile;
    bfFile = fopen(fileName,"r");
    if(bfFile==NULL){
        printf("ERROR : FILE %s DOES NOT EXIST\n",fileName);
        return 2;
    }
    char c;

    //c source code
    FILE* cFile;
    cFile = fopen(outFileC,"w");
    int add = 0;
    char prevC = '\0';
    fputs("#include<stdio.h>\n#include<stdlib.h>\n",cFile);
    fputs("int main(){",cFile);
    fputs("unsigned char* _=(unsigned char*)malloc(32*1024);/*32kB*/if(_==0){printf(\"MEMORY ERROR!\\n\");return 1;}",cFile);
    //write codes   
    do{
        c = fgetc(bfFile);
        if(c!=EOF){
            bool isPrint = false;
            switch(c){
                case '>':
                case '<':
                    if((prevC=='+'||prevC=='-')&&add){//process something like +++- = ++
                        if(add==1||add==-1){
                            if(add==1) fputs("++*_",cFile);
                            else fputs("--*_",cFile);
                        }
                        else fprintf(cFile,"(*_)%c=%d",add>0?'+':'-',add>0?add:-add);
                        fputs(";",cFile);
                        add=0;
                    }
                    if(c=='>') add++;
                    else add--;
                    break;
                case '+':
                case '-':
                    if((prevC=='>'||prevC=='<')&&add){//process something like >>>< = >>
                        if(add==1||add==-1){
                            if(add==1) fputs("++_",cFile);
                            else fputs("--_",cFile);
                        }
                        else fprintf(cFile,"_%c=%d",add>0?'+':'-',add>0?add:-add);
                        fputs(";",cFile);
                        add=0;
                    }
                    if(c=='+') add++;
                    else add--;
                    break;
                case '.':
                case ',':
                case '[':
                case ']':
                    if(add){
                        if(prevC=='>'||prevC=='<'){//process something like >>>< = >>
                            if(add==1||add==-1){
                                if(c=='.'){
                                    isPrint = true;
                                    if(add==1) fputs("putchar(*(++_))",cFile);
                                    else fputs("putchar(*(--_))",cFile);
                                }else{
                                    if(add==1) fputs("++_",cFile);
                                    else fputs("--_",cFile);
                                }
                            }
                            else if(c=='.'){
                                isPrint = true;
                                fprintf(cFile,"putchar(*(_%c=%d))",add>0?'+':'-',add>0?add:-add);
                            }else fprintf(cFile,"_%c=%d",add>0?'+':'-',add>0?add:-add);
                            fputs(";",cFile);
                            add=0;
                        }else if(prevC=='+'||prevC=='-'){//process something like +++- = ++
                            if(add==1||add==-1){
                                if(c=='.'){
                                    isPrint = true;
                                    if(add==1) fputs("putchar(++*_)",cFile);
                                    else fputs("putchar(--*_)",cFile);
                                }else{
                                    if(add==1) fputs("++*_",cFile);
                                    else fputs("--*_",cFile);
                                }
                            }
                            else if(c=='.'){
                                isPrint = true;
                                fprintf(cFile,"putchar((*_)%c=%d)",add>0?'+':'-',add>0?add:-add);
                            }else fprintf(cFile,"(*_)%c=%d",add>0?'+':'-',add>0?add:-add);
                            fputs(";",cFile);
                            add=0;
                        }
                    }
                    switch(c){
                        case '.':
                            if(!isPrint) fputs("putchar(*_);",cFile);
                            break;
                        case ',':
                            fputs("*_=getchar();",cFile);
                            break;
                        case '[':
                            fputs("while(*_){",cFile);
                            break;
                        case ']':
                            fputs("}",cFile);
                            break;
                    }
                    break;
            }
        }
        if(c=='>'||c=='<'||c=='+'||c=='-'||c=='.'||c==','||c=='['||c==']') prevC = c;
    }while(c!=EOF);
    fputs("free(_);return 0;}",cFile);
    fclose(bfFile);
    fclose(cFile);
    if(!doCompile){
        printf("Output C code : %s\n",outFileC);
        return 0;
    }
    printf("Compile with GCC...\n");
    char op[2048] = "gcc ";
    strcat(op,outFileC);
    strcat(op," -o ");
    strcat(op,outFileName);
    system(op);
    if(deleteSource){
        strcpy(op,"rm ");
        strcat(op,outFileC);
        system(op);
    }
    printf("Done.\nOutput file name : %s\n",outFileName);
    return 0;
}

Answer 1

1. Don't put everything in a giant main function, that just makes it hard to follow
2. Print error message, such as usage information to stderr
3. You have argc (argument count) and argv (argument vector) backwards
4. You don't free anything you malloc
5. There is no point in mallocing, because you can create your arrays on the stack
6. Rather then passing a significantly lower value as your size to strncpy, use strncat for the concatenation.
7. Repeated constants like the size of the strings you are using are best made #defines
8. Since you appear to be using a recent version of C, don't split definitions and assignments onto seperate lines unneccesairly
9. I prefer putting logic into else blocks rather then depending on return skipping the rest.
10. Rather then calling rm, use the standard library function "unlink" for deleteing files
11. Don't generate code using ++/-- it would won't help the final speed and makes your code more complicated
12. Don't try to subtract instead of adding negative numbers, the compiler is smart enough to handle that.
13. Why isn't the semicolon inside the printf?
14. The name add isn't very clear. It took me a while to figure out what it was doing
15. In general you spend a lot of effort trying to generate more compact C code. But the C compiler will be smarter at doing that then you are, so don't try.

Having cleaned up those issues I can see the problematic repetition of logic. The first thing is that all of non-bf characters are causing complications. We'll write function which calls fgetc but filters out all the characters we want to ignore.

Next, rather then trying to respond one character at a time, have loops which eat the characters.

Here is the result:

#include<stdio.h>
#include<stdbool.h>
#include<string.h>
#include<stdlib.h>

#define STRING_SIZE 1024

typedef struct CommandOptions
{
    bool doCompile;
    bool deleteSource;
    char * fileName;
    char outFileName[STRING_SIZE];
    char outFileC[STRING_SIZE];
}CommandOptions;

bool parse_command_line(CommandOptions * options, int argc, char * argv[])
{
    //help message
    if(argc==1){
        fprintf(stderr, "\n");
        fprintf(stderr, "How to use : %s filename [-o output|-c|-d|-r]\n",argv[0]);
        fprintf(stderr, "-o output : Set output file name\n-c : Do not compile\n-d : Do not delete C source file\n");
        fprintf(stderr, "INFO : You SHOULD type 'export MALLOC_CHECK_=0' manually to remove warning.\n");
        fprintf(stderr, "\n");
        return false;
    }

    // set the default options
    options->doCompile = true;
    options->deleteSource = true;
    options->fileName = argv[1];

    strncpy(options->outFileName, options->fileName, STRING_SIZE);
    strncat(options->outFileName, ".o", STRING_SIZE);

    // parse the remaining options
    int i;
    bool isSetOut = false;

    for(i = 0; i < argc; i++) 
    {
        if(isSetOut){
            isSetOut = false;
            strncpy(options->outFileName, argv[i], STRING_SIZE);
        }
        else if(strcmp(argv[i],"-c")==0){
            options->doCompile = false;
        }
        else if(strcmp(argv[i],"-d")==0){
            options->deleteSource = false;
        }
        else if(strcmp(argv[i],"-o")==0){
            isSetOut = true;
        }
    }

    strncpy(options->outFileC,options->outFileName,STRING_SIZE);
    strncat(options->outFileC,".c", STRING_SIZE);

    // don't delete the source if we won't compile it
    if(!options->doCompile)
    {
        options->deleteSource = false;
    }

    return true;

}

void write_header(FILE * cFile)
{
    fputs("#include<stdio.h>\n", cFile);
    fputs("#include<stdlib.h>\n",cFile);
    fputs("int main(){\n",cFile);
    fputs("unsigned char* _=(unsigned char*)malloc(32*1024);/*32kB*/if(_==0){printf(\"MEMORY ERROR!\\n\");return 1;}\n",cFile);
}

void write_footer(FILE * cFile)
{
    fputs("free(_);\nreturn 0;\n}\n",cFile);
}

int bf_fgetc(FILE * bfFile)
{
    int c;
    do
    {
        c = fgetc(bfFile);
    }while(c != EOF && c != '[' && c != ']' && c != '<' && c != '>' && c != '.' 
        && c != ',' && c != '+' && c != '-');
    return c;
}

void compile_to_c(FILE * bfFile, FILE * cFile)
{
    write_header(cFile);
    int add = 0;
    char prevC = '\0';
    //write codes   
    char c = bf_fgetc(bfFile);
    do{
        int movement_counter = 0;
        while( c == '>' || c == '<')
        {
            movement_counter += c == '>' ? 1 : -1;
            c = bf_fgetc(bfFile);
        }
        if(movement_counter)
        {
            fprintf(cFile,"_ += %d;", movement_counter);
        }

        int value_counter = 0;
        while( c == '+' || c == '-')
        {
            value_counter += c == '+' ? 1 : -1;
            c = bf_fgetc(bfFile);
        }
        if(value_counter)
        {
            fprintf(cFile,"*_ += %d;",value_counter);
        }

        if(c == '.') 
        {
            fprintf(cFile, "putchar(*_);\n"); 
            c = bf_fgetc(bfFile); 
        }
        if(c == ',') 
        {
            fprintf(cFile, "*_ = getchar();\n");
            c = bf_fgetc(bfFile); 
        }
        if(c == '[')
        {
            fprintf(cFile, "while(*_) {\n");
            c = bf_fgetc(bfFile);
        }
        if(c == ']')
        {
            fprintf(cFile, "}\n");
            c = bf_fgetc(bfFile);
        }
    }while(c!=EOF);
    write_footer(cFile);
}

void compileCode(CommandOptions * options)
{
    printf("Compile with GCC...\n");
    char op[2048] = "gcc ";
    strncat(op,options->outFileC, 2048);
    strncat(op," -o ", 2048);
    strncat(op,options->outFileName, 2048);
    system(op);
}

int main(int argc, char* argv[]){
    CommandOptions options;

    if( !parse_command_line(&options, argc, argv) )
    {
        return 1;
    }

    printf("INFO : You may type 'export MALLOC_CHECK_=0' manually to remove warning.\n");

    //bf file
    FILE* bfFile = fopen(options.fileName,"r");
    if(bfFile==NULL){
        fprintf(stderr, "ERROR : FILE %s DOES NOT EXIST\n",options.fileName);
        return 2;
    }

    //c source code
    FILE* cFile = fopen(options.outFileC, "w");
    if(!cFile)
    {
        fclose(bfFile);
        fprintf(stderr, "ERROR: COULD NOT OPEN %s FILE FOR WRITING" , options.outFileC);
        return 3;
    }

    compile_to_c(bfFile, cFile);

    fclose(bfFile);
    fclose(cFile);

    if(options.doCompile){
        compileCode(&options);
    }else{
        printf("Output C code : %s\n", options.outFileC);
    }

    if( options.deleteSource )
    {
        unlink(options.outFileC);
    }

    printf("Done.\nOutput file name : %s\n",options.outFileName);
    return 0;
}

There is still some duplication in the compile_to_c function that I'm not liking. The similarity between <> and +- could easily be exploited easily in a function. Harder would be getting rid of the repeated bf_fgetc calls.

Answer 2

Don't cast malloc. It's unnecessary in C, and can hide a bug if you don't have a valid prototype in scope (ie. you didn't include stdlib.h)