C programming language, memory problems

Question

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;
}

After a quick look I see that malloc(sizeof(char)) allocates memory for a single char so your fscanf calls overflow city1 and city2. — Blastfurnace
– Blastfurnace, Commented 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. — Blastfurnace
– Blastfurnace, Commented Jun 9, 2012 at 6:07

Hari Menon · Accepted Answer · 2013-03-07 05:44:08Z

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[]

edited Mar 7, 2013 at 5:44

answered Jun 9, 2012 at 5:55

Hari Menon

35.7k15 gold badges90 silver badges111 bronze badges

Sign up to request clarification or add additional context in comments.

1 Comment

bledi Over a year ago

oh thanks man.Thank you very much. It now worked. God bless you. I had hours trying to find my mistake..

hobbs · Accepted Answer · 2012-06-09 05:56:46Z

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.

answered Jun 9, 2012 at 5:56

hobbs

245k20 gold badges225 silver badges304 bronze badges

Comments

paulsm4 · Accepted Answer · 2012-06-09 05:57:54Z

1

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

http://linux.die.net/man/3/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.

answered Jun 9, 2012 at 5:57

paulsm4

123k23 gold badges175 silver badges245 bronze badges

Comments

steveha · Accepted Answer · 2012-06-09 06:04:26Z

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 *)).

edited Jun 9, 2012 at 6:04

answered Jun 9, 2012 at 5:58

steveha

77.1k21 gold badges94 silver badges119 bronze badges

2 Comments

Blastfurnace Over a year ago

If the city names can be 20 characters long he/she should malloc(21) to allow for the null terminator.

steveha Over a year ago

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.

Collectives™ on Stack Overflow

C programming language, memory problems

4 Answers 4

1 Comment

Comments

Comments

2 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

4 Answers 4

1 Comment

Comments

Comments

2 Comments

Your Answer

Sign up or log in

Post as a guest

Related