Can't find error in string manipulation/pointers
So first off, this is a homework assignment, so please don't write any code for me, just point out where my code is wrong.
The basics of the code is that it's a 'address/balance' book. I've got a struct with my variables, but for some reason, my doubly linked list is getting all messed up and I can't figure out how. With a bit of fancy (not really) debugging, I've figured out that I think the fgets(string, 200, file); line is overwriting my head->name pointer somehow, which seems to throw the rest of the code off. Relevant code snippets here:
Populating the List:
void populate_list(char* filename){
FILE *file = NULL;
char* name = NULL;
char* streetaddress = NULL;
char* city = NULL;
char* state = NULL;
char line[200];
int zip;
float balance;
file = fopen(filename,"r");
if(file==NULL){
printf("Invalid input file\n");
exit(1);
}
if(file == 0){
printf("Invalid file.\n");
exit(1);
}
while(!feof(file)){
fgets(line, 200, file);
name = strtok(line, ",");
streetaddress = strtok(NULL, ",");
city = st开发者_运维问答rtok(NULL,",");
state = strtok(NULL,",");
zip = atoi(strtok(NULL,","));
balance = atof(strtok(NULL,","));
strip_spaces(name);
strip_spaces(streetaddress);
strip_spaces(city);
strip_spaces(state);
add_node(name, streetaddress, city, state, zip, balance);
}
fclose(file);
return;
}
Then the add_node code:
void add_node(char* name, char* streetaddress, char* city, char* state, int zip, float, balance){
struct customer* addnode = NULL;
if(find_duplicate(name)){
print_filler(1);
printf("DUPLICATE RECORD: %s\n", name);
return;
} else {
addnode = (struct customer *) malloc(sizeof(struct customer));
addnode->name = name;
addnode->streetaddress = streetaddress;
addnode->city = city;
addnode->state = state;
addnode->zip = zip;
addnode->balance = balance;
if(head == NULL) {
head = addnode;
addnode->prev = NULL;
} else {
tail->next = addnode;
addnode->prev = tail;
}
tail = addnode;
addnode->next = NULL;
}
print_list();
return;
}
The bug seems to be happening after the first time add_node is called and happens when fgets() goes off a second time. It's overwriting head->name with the entire fgets() line for some reason.
There other random irritating bugs I haven't pinned down yet, but this one I think may be the source of the others.
Full code is here, if it helps: http://pastebin.com/k0pqyvT0
My guess is it's something to do with the head->name pointer being overwritten by fgets(), but I can't for the life of me figure out what exactly is doing this, any help/advice would be appreciated.
Edit: Solution was to implement strdup() and duplicate the strings when they're passed to add_node(). Thanks for all the answers.
- The variable "string" is a pointer to nothing. You need to actually allocate a buffer into which you read the data.
- strtok returns a pointer to the original string, which then changes. You need to allocate a new buffer and copy the string into it if you wish to keep it around in your customer structure.
The main problem is that you are assigning the members of the node you allocate to point into the line
buffer, and that buffer is overwritten every time you call fgets()
to fill it. So the pointers in every node always point into same buffer (which lives on the stack).
The solution (as mentioned in other answers) is to use strdup()
to allocate a separate copy of each string that you're saving to each node member. So something like this would work:
....
} else {
addnode = (struct customer *) malloc(sizeof(struct customer));
addnode->name = strdup(name);
addnode->streetaddress = strdup(streetaddress);
addnode->city = strdup(city);
addnode->state = strdup(state);
addnode->zip = strdup(zip);
addnode->balance = strdup(balance);
...
}
It's also a good idea to zero-out the contents of the node immediately after it has been allocated, just so you don't get garbage in it:
addnode = (struct customer *) malloc(sizeof(struct customer));
memset(addnode, '\0', sizeof(struct customer));
I don't know if it's related, but your pointer struct customer* addnode = NULL;
is a local inside add_node(), it won't get saved (i.e. every time the function exits, it gets destroyed). Also be careful when using pointer to char, it may seems to be intuitively a string, but there's a gotcha,
So if you do things like:
while(!feof(file)) {
name = strtoke(string, ",");
// etc
add_node(name, ... //etc);
}
since you only copy reference i.e. addnode->name = name
, the next line read will replace the content of same pointer used as the previous record, thus you'll have list with same properties.
As Dark Falcon points out, you *add_node* code is not correct. You need to do something like this instead:
void add_node(char* name, char* streetaddress, char* city, char* state, int zip, float balance){
struct customer* addnode = NULL;
strip_spaces(name);
strip_spaces(streetaddress);
strip_spaces(city);
strip_spaces(state);
if(find_duplicate(name)){
print_filler(1);
printf("DUPLICATE RECORD: %s\n", name);
return;
} else {
addnode = (struct customer *) malloc(sizeof(struct customer));
addnode->name = name;
addnode->streetaddress = streetaddress;
addnode->city = city;
addnode->state = state;
addnode->zip = zip;
addnode->balance = balance;
if(head == NULL) {
head = addnode;
addnode->prev = NULL;
} else {
tail->next = addnode;
addnode->prev = tail;
}
tail = addnode;
addnode->next = NULL;
}
print_list();
return;
}
and then call it like this:
add_node(strdup(name), strdup(streetaddress), strdup(city), strdup(state), zip, balance);
精彩评论