2

I'm trying to read a file text(input.txt) which consists of multiple line like A B 120 C B 60...
Well, now I'm trying to transfer the names that may have been repeated more than once in that file into a double pointer where they should be shown only once. In my code below I get some of them but I also get a segmentation fault. I don't know what I am missing or what's wrong. Any little help of yours could help me very much.

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
int main(int argc, char *argv[]){
   int i=1, state=0, k, dist;
int** myMat;
char *city1, *city2, **matnames;
FILE* p;
    city1 = (char*) malloc(sizeof(char));
    city2 = (char*) malloc(sizeof(char));
    matnames = (char**) malloc(sizeof(char*));
    myMat = (int**) malloc(sizeof(int*));               
p = fopen(argv[1],"r");
/************************************************************/
    matnames[0] = (char*) malloc(sizeof(char));
    matnames[1] = (char*) malloc(sizeof(char));
    matnames[2] = (char*) malloc(sizeof(char));
    matnames[2] = NULL;
    fscanf(p, "%s %s %d", city1, city2, &dist);
        strcpy(matnames[0],city1);
        strcpy(matnames[1],city2);
/************************************************************/  
while( fscanf(p,"%s %s %d",city1,city2, &dist) != EOF){             
        for(k=0; matnames[k]!=NULL; k++){                   
            if( strcmp(matnames[k], city1) != 0){
                 state++;
                }               
        }       
        if(state  == k){
            matnames[k] = (char*) malloc(sizeof(char));
            strcpy(matnames[k], city1);
            matnames[k+1] = (char*) malloc(sizeof(char));
            matnames[k+1] = NULL;
            }
        state = 0;
        for(k=0; matnames[k] != NULL;k++){                  
                if( strcmp(matnames[k], city2) != 0){
                    state++;
                }               
        }
        if(state == k){
            matnames[k] = (char*) malloc(sizeof(char));
            matnames[k+1] = (char*) malloc(sizeof(char));
            strcpy(matnames[k], city2); 
            matnames[k+1] = NULL;   
            }
        state = 0;

}
return 0;
}
2
  • After a quick look I see that malloc(sizeof(char)) allocates memory for a single char so your fscanf calls overflow city1 and city2. Jun 9, 2012 at 5:57
  • I also don't see any calls to free() or fclose(). You should practice better resource management to avoid leaking memory or file handles. Jun 9, 2012 at 6:07

4 Answers 4

3
matnames = (char**) malloc(sizeof(char*));

You allocated enough memory for just one char* here.

Then you are calling

matnames[1] = (char*) malloc(sizeof(char));

whereas matnames has space only for one char*, matnames[1] is trying to access space for a second char *.

What you need to do is

matnames = (char**) malloc(n * sizeof(char*));

where n is the number of elements you expect to store in matnames[]

1
  • oh thanks man.Thank you very much. It now worked. God bless you. I had hours trying to find my mistake..
    – bledi
    Jun 9, 2012 at 6:00
1

Among other things, you've allocated enough memory for matnames to store one char *, but you're storing three values in it. It also doesn't help that you malloc some memory for matnames[2] and then immediately overwrite the malloc'd pointer with NULL — but that's just a memory leak, since you don't actually try to store anything into *(matnames[2]). There may be other problems, but let's start with the most immediate one.

0
1

You might also be interested in "strdup()":

It combines "malloc()" and "strcpy()" into one operation - and saves you from worrying about whether or not you've "malloc'ed" the correct size.

1

In C, the expression sizeof(char) will always return 1. This is by definition: sizeof() can work on many different kinds of computer, and not every one uses bytes.

So you are allocating only one byte for each name. If the city names can be 20 characters long, use malloc(21) (don't forget to also have space for the terminating '\0' in the string).

For the (char **) pointer, you should figure out how many pointers you will need to store, and multiply by sizeof(char *). So, if you need to store 3 char * pointers, then use malloc(3 * sizeof(char *)).

2
  • If the city names can be 20 characters long he/she should malloc(21) to allow for the null terminator. Jun 9, 2012 at 5:59
  • Yes, you are right. I swear I wouldn't make that mistake at work, but I'm pretty tired right now. I'll edit the answer to make the point: don't forget the NUL.
    – steveha
    Jun 9, 2012 at 6:03

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.