Segmentation fault when reading arguments from command line

Question

I am trying to implement a duplicate cat function in c. I am getting segmentation faults, and I am unable to find why.

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




int main(int argc,char *argv[])
{
    char* s;        /* input string */

    int c;

    if(argc==1){


        while (gets(s)){
            puts(s);
        }
    }

    else{

    FILE *file = fopen( "./argv[1]", "r" );
    while((c=fgetc(file))!=EOF){
        fputc(c,stdout);

    }

    fclose(file);   

    }


    return 0;
}

First of all, you should debug your program. If you don't know how to do that this is a perfect time learn. This will almost always give you the line of the error immediately and thus save you from long searches. Second, you never allocated memory for s. And last, you want to open `./argv[1]`, not `"./argv[1]"`. — Zeta, Apr 16 '13 at 06:30
Ah yes the daily I-write-stuff-to-an-address-where-nothing-is-allocated question came early today. Too localized, vote to close. — Lundin, Apr 16 '13 at 06:31
@Lundin Yeah, I'm afraid there are yet more of this question in today's daily routine on SO... -.- — , Apr 16 '13 at 06:33
I'd use `fread` instead of `fgets`, as `cat` does not actually need to work using newlines, and that way we avoid special handling for long lines. — Keith, Apr 16 '13 at 06:53

Bart Friederichs · Answer 1 · 2013-04-16T06:35:55.297

5

You are reading characters into s without allocating memory:

    while (gets(s)){
        puts(s);
    }

Allocate some memory first by using malloc (you will have to free it later):

    char *s = malloc(some_buffer_size);

or make it fixed:

    char s[some_buffer_size];

Be careful with gets by the way, as there is no way of limiting the number of characters input, which can lead to buffer overflows. Use fgets(buf, sizeof(buf), stdin) instead.

Second mistake has been pointed out by others, you cannot call fopen like that, use this instead, or use the solution from H2CO3:

 FILE *file = fopen( argv[1], "r" );

edited Apr 16 '13 at 06:35

answered Apr 16 '13 at 06:30

Bart Friederichs

33,050
15
95
195

Also `fopen( "./argv[1]", "r" );` note the first argument – Kiril Kirov Apr 16 '13 at 06:31
1

Please [don't cast the return value of `malloc()` in C](http://stackoverflow.com/a/605858/28169). – unwind Apr 16 '13 at 06:31
Oh yeah. Thanx. But now I am getting segmentation faul when trying to read from file – Assasins Apr 16 '13 at 06:33
Using `gets` in the first place is probably more severe a fault than casting the result of `malloc`. `gets` is not part of the C language no longer, don't use it. – Lundin Apr 16 '13 at 06:33
2

@Fazlan Because your `fopen()` returns `NULL`. Time to read a C book and to develop some common sense. – Apr 16 '13 at 06:33
1

No reason not to use a fixed buffer `char s[some_buffer_size]`. – Keith Apr 16 '13 at 06:33
@Keith That's exactly what was going to be my second comment. – Apr 16 '13 at 06:34
@H2CO3 that's what I suggest – Bart Friederichs Apr 16 '13 at 06:36
@BartFriederichs Yap, didn't see the `f`... Sorry. – Apr 16 '13 at 06:37

score 1 · Answer 2 · answered Apr 16 '13 at 06:30

1

gets(s); - Here s is not initialized to point to anything. fopen( "./argv[1]", "r" ); - This is wrong. Must be - fopen(argv[1], "r" );

answered Apr 16 '13 at 06:30

Superman

3,027
1
15
10

score 0 · Answer 3 · answered Apr 16 '13 at 06:31

fopen( "./argv[1]", "r" );

Huh, what? Assumptions, right?

char buf[0x1000];
snprintf(buf, sizeof(buf), "./%s", argv[1]);
FILE *f = fopen(buf, "r");

Also, you are using the s pointer uninitialized (you don't allocate any memory for it), so your program also invokes undefined behavior.

score 0 · Answer 4 · answered Apr 16 '13 at 06:49

0

malloc() basically allocates memory. in the end, you need to free() your variable as well to free the allocated space.

you should use gdb() for finding out errors and debugging your program.

answered Apr 16 '13 at 06:49

Segmentation fault when reading arguments from command line

4 Answers4