Implementing 'cat' in c - wwwmc? (What's wrong with my code)

Question

My code works fine in a way. I have two issues with it though. One, at the end of printing the files to the standard output, it gives me a segmentation fault.

Two, I can't use fputs to print out the data because I get a segmentation fault right away. So to fix it I use puts which prints it fine, but adds a '\n' after every line making the text single line spaced as well as the seg fault at the end.

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

void concat(char *arg){

    char string[256];
    FILE *fp = fopen(arg, "r");

    while(!feof(fp)){
        fgets(string, 256, fp);
        //fputs(string, fp);
        puts(string);
    }

    fclose(fp);

}

void stdincat(){

    char string[256];   
    while(!feof(stdin)){
        fgets(string, 256, stdin);
        fputs(string, stdout);
    }
}

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

    char argvalues[256][40];

    if(argv[1] == NULL)
        stdincat();
    else if(argv[1] != NULL){
        int i;

        for(i=1;i<=(argc);i++){
            concat(argv[i]);
        }
    }

    return 0;
}

Answer 1

The call to fputs you have commented out in concat is attempting to write to fp, which you opened only for reading, so it's no surprise that it won't/doesn't work.

Other than that, your reading loops: while(!feof(fp)) { (and similar except from stdin instead of fp) follow a familiar, widespread anti-pattern -- loops like this don't/won't/can't work correctly. You normally want to read and test for success in the same operation:

while(fgets(string, 256, stdin))
    fputs(string, stdout);

Edit: I should also mention that I would prefer to avoid the duplication of code in concat and stdincat. I'd rather pass the FILE * to read from as the parameter, so you'd use the same code to read from stdin or from other arbitrary files:

// Warning: untested code.
// error checking omitted for readability.
void catfile(FILE *file) { 
    char line[256];
    while (fgets(line, sizeof(line), file))
        puts(line);
}

int main(int argc, char **argv) { 
    int i;
    if (argc == 1)
        catfile(stdin);
    else for (int i=1; i<argc; i++) {
        FILE *infile = fopen(argv[i], "r");
        catfile(infile);
        fclose(infile);
    }
    return 0;
}

Finally, I'd note that if you're just going to copy an entire file, fgets probably is not the most efficient way to do the job. fread may be more suitable. While you're at it, you might as well use binary mode reading and writing as well.

Answer 2

You cannot write on the stream that you opened only for read, you should use stdout for the fputs in the function concat