Working is not the same as semantically correct. You are basically writing a reversed linked list.
What do you mean you don't need to? You need to free everything you malloc. I don't see a single free statement in the code.
Working is not the same as semantically correct. You are basically writing a reversed linked list.
What do you mean you don't need to? You need to free everything you malloc. I don't see a single free statement in the code.
Originally Posted by Adak
haha, all right.
I can not retort with any excuse that I did not free the malloc
And one more question:
How could I know if my malloc has been freeed ?
with code below ( no more argue about the way of inserting, only concern the free malloc problem)
Code:#include <stdio.h> #include <stdlib.h> struct list_el { int val; struct list_el *next; }; typedef struct list_el item; int main(int argc, char **argv) { item *curr, *head,*tmp; int i; curr = head = tmp = NULL; for(i=1; i<=5; i++) { curr = (item*)malloc(sizeof(item)); curr->val = i; curr->next = head; head = curr; } head = curr; while(curr) { printf("\n %d",curr->val); tmp = curr; curr = curr->next; free(tmp); } return 0; }
Easiest way is to use a tool such as valgrind. It will detect memory leaks.
If I try to free memory as in shown in the snippet, I get warning stating "Invalid Address specified to RtlFreeHeap" and "Program Received signal SIGTRAP".
Can anybody suggest correct way of freeing memory ??
Code:/*Program constructs an linked list of 5 members and displays it */ #include <stdio.h> #include <stdlib.h> struct node { int x; node *next; }; int main(int argc, char **argv) { struct node *head, *curr, *temp; int i; head = (struct node*)malloc(sizeof(struct node)); curr = head; for(i=1;i<=5;i++) { curr -> next = (struct node*)malloc(sizeof(struct node)); curr -> x= i; temp=curr; curr = curr->next; } temp->next =NULL; //Display members curr = head; while(curr->next!=NULL) { printf("\n %d", curr->x); curr = curr->next; temp = curr; free(temp); } printf("\n %d", curr->x); // to print the last element in the list return 0; }
Statements are mixed up.Code:curr = curr->next; temp = curr;
You're advancing the pointer, freeing it, then on the next iteration, you're trying to access the very same pointer you just freed.
switch the order of statements below:
I tried the whole program, looks fine.Code:curr = curr->next; temp = curr;
Code:/*Program constructs an linked list of 5 members and displays it */ #include <stdio.h> #include <stdlib.h> struct node { int x; struct node *next; }; int main(int argc, char **argv) { struct node *head, *curr, *temp; int i; head = (struct node*)malloc(sizeof(struct node)); curr = head; for(i=1;i<=5;i++) { curr -> next = (struct node*)malloc(sizeof(struct node)); curr -> x= i; temp=curr; curr = curr->next; } temp->next =NULL; //Display members curr = head; while(curr->next!=NULL) { printf("\n %d", curr->x); temp = curr; curr = curr->next; free(temp); } printf("\n %d", curr->x); // to print the last element in the list return 0; }
But if I try to execute this program I get "Program Received signal SIGTRAP" warnig and I do not get any output?
So my question is, should I apply another loop to free the memory, I mean some thing llike below snippet?
With this loop, I got the output but debugger hangs and I still receive the warning stating"Program Received signal SIGTRAP".
Can anbody please suggest any solution to this problem?
Code:curr = head; while(curr->next!= NULL) { free(curr); curr = curr->next; }
Once you free(curr), it follows that you should not write curr->next.
Furthermore, it may well be the case that curr is a null pointer, hence writing curr->next in the loop condition could be wrong.
Rather think along the lines of: while curr is not a null pointer, get the next pointer, then free curr, and then make the next pointer curr... rinse, wash, repeat.
Look up a C++ Reference and learn How To Ask Questions The Smart Way
Do you mean something like this:
I tried this way, I mean I get output along with the warning"Program Received signal SIGTRAP" and debugger hangs. Is this an right way of freeing memory ?Code:curr =head; while(curr!= NULL) { curr =curr->next; free(curr); } free(curr); // free the tail curr = head; free (curr);
You're close, but think: if you assign curr->next to curr, then free curr, what you're freeing would be the original curr->next, not the actual curr that you want to free.
Look up a C++ Reference and learn How To Ask Questions The Smart Way
This works :
Thanks for the help.Code:curr = head; while(curr!= NULL) { temp =curr->next; free(curr); curr = temp; } free(curr); // free the tail
Yes, that works. The last free(curr) is probably unnecessary, though.
I think it is necessary since the while loop is curr != NULL, so it is not going to free the last member with curr->next =NULL.
No. In order for the loop to break out the condition must be false, therefore curr will be NULL. Freeing NULL is not advisable.
1. Get rid of gets(). Never ever ever use it again. Replace it with fgets() and use that instead.
2. Get rid of void main and replace it with int main(void) and return 0 at the end of the function.
3. Get rid of conio.h and other antiquated DOS crap headers.
4. Don't cast the return value of malloc, even if you always always always make sure that stdlib.h is included.
If curr != NULL, then it will save curr->next in curr and free curr.
If cur->next != NULL at this point, then curr != NULL on the next loop, and hence it will be freed to.
If cur->next == NULL, the loop terminates and cur == NULL.
According to that logic (I could be wrong), it should not be necessary to use the last free.
