Spliting the string and displaying it....

Question

Actually,the following program is to split the string and display the output.I have done my program and it is giving the desired output.But,at the end it is giving segmentation fault.I tried with debugger also.But,I was unable to find the problem.can anyone help me?

Input File:

 
  Vivek|Raj|20
  Abi|Nila|20 
  

Expected Output:

  Vivek's last name Raj and age is 20.
  Abi's last name Nila and age is 20.

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

struct person{
        char firstname[30];
        char lastname[30];
        int age;
};
int main(int args, char *argv[])
{
        struct person list[30];
        int i,n;
        char *ch,*ch1,*ch2,*ch3;
        char buf[256];
        FILE *fp;

        fp = fopen(argv[1],"r");
        if(fp == NULL){
                printf("Cannot open file %s\n", argv[1]);
                exit(0);
        }
        i=0;
        fgets(buf,256,fp);
        while(!feof(fp)){
                ch = strchr(buf,'\n');
                if(ch != NULL) *ch = '\0';
                else break;
                ch = strchr(buf,'|');
                ch2 = strrchr(buf,'|');
                if(ch != NULL)
                {
                        *ch = '\0';ch1 = ++ch2;
                }
                else break;
                ch++;
                strcpy(ch3,ch);
                ch2=strchr(ch3,'|');
                *ch2='\0';
                strcpy(list[i].firstname,buf);
                strcpy(list[i].lastname,ch3);
                list[i].age = atoi(ch1);
                i++;
                fgets(buf,256,fp);
        }
        n = i;

        for (i = 0; i < n; i++)
                printf("%s's last name %s and age.\n",list[i].firstname,list[i].lastname,list[i].age);
        return 0;
}

Answer 1

As a first point, you're doing the reading wrong. You should loop on fgets(), since it returns NULL on failure (which, typically, means you've reached end of file):

char buf[256];

while(fgets(buf, sizeof buf, fp))
{
  /* process each line here */
}

Answer 2

There are many reason why your code may crash.

1) It seems to me that ch3 isn't initialised at all. So it probably contains garbage and points (by chance) to a writable memory (maybe stack?). You should assign a pointer to a valid buffer to ch3 before using it. I think that the crash is due to this issue. A possible correction (not at all the best one) is:

 char tmp[256];
 char *ch3;

 // Make ch3 point to a valid buffer
 ch3 = tmp;

 // ...
     // Here ch3 must point to a valid location!!!
     strcpy(ch3,ch);

2) By the way, you don't need a second buffer at all: you can look for the two separators, substitute them with a termination character and take directly the pointers to the three strings from the original buffer:

            ch = strchr(buf,'|');
            if (ch == NULL) 
            {
            // INPUT MALFORMED
                    break;
            }
            *ch = '\0'; // This splits first and second+third
            ++ch;
            // Now ch points to the second+third field!
            ch2 = strrchr(ch,'|');
            if(ch2 == NULL)
            {
            // INPUT MALFORMED
                    break;
            }
            *ch2 = '\0'; // split second and third
            ++ch2;
            // Now ch2 points to the third field!
            //
            printf("First name=%s Last name=%s age=%s\n",buf,ch,ch2);

3) the program is limited to 30 items, but the numer of elements actually inserted is not checked anywhere (something like

//...
++i;
if (i >= 30) {
  break;
}

And, of course, it would be nice if the size if "list" array was parametrized via macro.

4) "firstname" and "lastname" are char[SOME_SIZE]. Does all the input fields fit in SOME_SIZE-1?

In general, the program misses some "limit" and "input validation" controls (e.g. what if the numeric field is malformed? Ok, it won't crash, but... uhm...)

I hope this may help.

Regards

Answer 3

Your printf statement is incorrect:

for (i = 0; i < n; i++)
        printf("%s's last name %s and age.\n",list[i].firstname,list[i].lastname,list[i].age);
return 0;

You're missing the %d for the last argument. It should be:

for (i = 0; i < n; i++)
        printf("%s's last name %s and age is %d.\n",list[i].firstname,list[i].lastname,list[i].age);
return 0;

Answer 4

You are doing a strcpy(ch3, ch); where ch3 has not been allocated any memory, since it is not assigned to NULL also, your code overwrites some unknown memory causing memory corruption and eventually leading to crash.