need help solving a segmentation fault with file i/o in C

Question

I read the input file , which has 911 lines , and want to copy these 911 lines into the output file , after making a few changes to each line..

I am getting segmentation fault when i run this.. I dont know why.. can anyone please help..

#include<stdio.h>
void main()
{
    int i;
    FILE *fin,*fop;char* str;
    fin=fopen("atk561011.txt","r");
    if(fin=NULL) printf("ip err");
    fop=fopen("svmip.txt","w");
    if(fop=NULL) printf("op err");
    for(i=1;i<=911;i++)
    {
        fgets(str,150,fin);
        if((i>300&&i<=360)||(i>600&&i<=660)) 
            str[7]='1';
        else 
            str[7]='0';
        fputs(str+7,fop);
        putc('\n',fop);
    }
    fclose(fin);
    fclose(fop);
}

Answer 1

For a start, this is wrong:

if(fin=NULL)

Should be:

if (fin == NULL)

(the same goes for fop, of course). And if you didn't succeed opening the file - don't just print an error, exit, because what are you going to read from? Keep in mind that the output of printf is buffered and in case of a segfault you won't always see it at all, even if it ran before the fault.

Another thing: you don't allocate memory for str, yet write into it with fgets.

And another thing: reading a pre-defined amount of lines from the file is probably a bad idea. It's better to read from the input until there is an end of file, or until a required amount of lines has been read.

Answer 2

Your not allocating any space for the pointer str.

Change that to either char str[/*Max Length You expect 150? */] or allocate the buffer.

In the meantime your code is walking all over memory - thus the segmentation fault.

Answer 3

Here's the corrected code that should do it.

#include
#define MAX_BUF   150
void main()
{
    int i;
    FILE *fin,*fop;char* str;
    str = malloc((MAX_BUF * sizeof(char)) + 1);
    if (str == NULL){
       printf("Out of memory\n");
       exit(-1);
    }
    fin=fopen("atk561011.txt","r");
    if(fin == NULL){
        printf("ip err");
        exit(-2);
    }
    fop=fopen("svmip.txt","w");
    if(fop == NULL){
        printf("op err");
        exit(-3);
    }
    for(i=1;i<=911;i++)
    {
        fgets(str,150,fin);
        if((i>300&&i<=360)||(i>600&&i<=660)) 
            str[7]='1';
        else 
            str[7]='0';
        fputs(str+7,fop); 
        // What is that for? should it be
        // fputs(str, fop); ????? 
        // since you're outputting the 7'th character (1/0)?
        putc('\n',fop);
    }
    fclose(fin);
    fclose(fop);
    if (str != NULL) free(str);
}

I have added logic checking to ensure if the file exists, to continue processing. Failure to do so will cause the code to blow up. Since in the original code, you were printing "ip err" if the input failed or in the case of output failure, yet continue on into the for loop which will render the execution to fail in that case as it would still be attempting to read from an non-existant file handle on failure.

Edit: Please see the comment above in the code. Are you trying to output 1/0's based on the conditional value of i between the ranges of 300-360 and 600-660 inclusively, into the output file. Can you clarify? Should that be

fputs(str[7], fop);

Hope this helps, Best regards, Tom.

Answer 4

Both fin=NULL and fop=NULL should both be using the 'equal-equal' operator. You're setting fin and fop to NULL instead of checking for a bad return value.