HELP, INFINIT LOOP... simple LINKED LIST

[na1paj]

here's a simple linked list program. the DeleteNode function is
producing an infinit loop i think, but i can't figure out where..
#include <stdio.h>
typedef struct
{
char *str; //str is a dynamic array of characters
int length; //number of characters
} String;

typedef struct node
{
String Data;
struct node* Link;
} ListNode;

typedef struct listStuct
{
ListNode *Head;
}List;

void AddNode(List *L, String item)
{
ListNode *currNode, *newNode;
currNode = L->Head;
if (L->Head == NULL)
{
L->Head = (ListNode*)mall oc(sizeof(ListN ode));
L->Head->Data = item;
L->Head->Link = NULL;
}
else
{
while(currNode->Link != NULL)
currNode = currNode->Link; //go to the last
newNode = (ListNode*)mall oc(sizeof(ListN ode));
newNode->Data = item;
newNode->Link = NULL;
currNode->Link = newNode;
}
}

void DeleteNode(List *L, String item)
{
ListNode *currNode, *prevNode;
currNode = L->Head;
prevNode = L->Head;
if (L->Head == NULL)
{
printf("empty list");
}
while(currNode != NULL) //check if first
{
if(strcmp(currN ode->Data.str,item. str)==0) //found
{
if(currNode == L->Head) //first one
{
L->Head == currNode->Link;
currNode->Link = NULL;
free(currNode);
printf("Remove First");
}
else
{
prevNode->Link = currNode->Link;
currNode->Link = NULL;
free(currNode);
printf("removed ");
}
}
else
{
prevNode = currNode;
currNode = currNode->Link; //go to next one
}
}
}
int main()
{
FILE *fp = NULL;
List list;
String string;
int i;
int records = 0;
char filename[100];
//list.Head = (ListNode*)mall oc(sizeof(ListN ode));
list.Head = NULL;
string.str = (char *)malloc(sizeof (char) *20);
printf("Enter filename\n");
scanf("%s", filename);
fp = fopen(filename, "r");

if (fp == NULL)
{ printf ("Error\n");
exit(0);
}
else
{
fscanf(fp, "%d", &records);
for (i = 0; i<records; i++)
{
fscanf(fp, "%s", string.str);
AddNode(&list, string);
printf("%s ", string.str);
}
printf("\n\nEnt er a Node to delete: ");
scanf("%s", string.str);
DeleteNode(&lis t, string);
}
return 1;
}

[Artie Gold]

na1paj wrote:

here's a simple linked list program. the DeleteNode function is
producing an infinit loop i think, but i can't figure out where..
#include <stdio.h>
typedef struct
{
char *str; //str is a dynamic array of characters
int length; //number of characters
} String;

typedef struct node
{
String Data;
struct node* Link;
} ListNode;

typedef struct listStuct
{
ListNode *Head;
}List;
Why bother? (Yes, it's a style issue, but having just a pointer to a
`ListNode' would be sufficient. The extra struct definition doesn't
really add anything.

void AddNode(List *L, String item)
{
ListNode *currNode, *newNode;
currNode = L->Head;
if (L->Head == NULL)
{
L->Head = (ListNode*)mall oc(sizeof(ListN ode)); Better:
L->Head = malloc( sizeof * L->Head );
L->Head->Data = item;
L->Head->Link = NULL;
}
else
{
while(currNode->Link != NULL)
currNode = currNode->Link; //go to the last
newNode = (ListNode*)mall oc(sizeof(ListN ode));
Similarly.
newNode->Data = item;
newNode->Link = NULL;
currNode->Link = newNode;
}
}

void DeleteNode(List *L, String item)
{
ListNode *currNode, *prevNode;
currNode = L->Head;
prevNode = L->Head;
if (L->Head == NULL)
{
printf("empty list");
}
while(currNode != NULL) //check if first
{
if(strcmp(currN ode->Data.str,item. str)==0) //found
{
if(currNode == L->Head) //first one
{
L->Head == currNode->Link;
currNode->Link = NULL;
free(currNode);
OK. Suppose you get here. What's the value of `currNode'?
[Hint: You're not even allowed to look at it. Doing so, as you will
when you get back to the top of the loop, invokes undefined
behavior. So all bets are off.]
printf("Remove First");
}
else
{
prevNode->Link = currNode->Link;
currNode->Link = NULL;
free(currNode);
printf("removed ");
}
}
else
{
prevNode = currNode;
currNode = currNode->Link; //go to next one
}
}
}
int main()
{
FILE *fp = NULL;
List list;
String string;
int i;
int records = 0;
char filename[100];
//list.Head = (ListNode*)mall oc(sizeof(ListN ode));
list.Head = NULL;
string.str = (char *)malloc(sizeof (char) *20);
printf("Enter filename\n");
scanf("%s", filename);
fp = fopen(filename, "r");

if (fp == NULL)
{ printf ("Error\n");
exit(0);
}
else
{
fscanf(fp, "%d", &records);
for (i = 0; i<records; i++)
{
fscanf(fp, "%s", string.str);
AddNode(&list, string);
printf("%s ", string.str);
}
printf("\n\nEnt er a Node to delete: ");
scanf("%s", string.str);
DeleteNode(&lis t, string);
}
return 1;
Erm, that's a non-standard return value; worse, a non-zero return
value indicates *failure*.
}

There are some other things wrong with the code, but I won't go into
that here. In general, though, the fact that you have special cases
to deal with typically indicates that your design is more complex
than it needs to be. Think about that -- and if you still run into
trouble, by all means repost.

HTH,
--ag
--
Artie Gold -- Austin, Texas
Oh, for the good old days of regular old SPAM.

[Richard Heathfield]

na1paj wrote:

here's a simple linked list program.
It's not as simple as you think.
the DeleteNode function is
producing an infinit loop i think, but i can't figure out where..
#include <stdio.h>
typedef struct
{
char *str; //str is a dynamic array of characters
If you have a C99 compiler, okay, fine. Do you? If not, // is a syntax
error.
int length; //number of characters
Were you planning on having a negative number of characters for any reason?

In fact, your code seems not to use the length field at all.
} String;

typedef struct node
{
String Data;
struct node* Link;
} ListNode;

typedef struct listStuct
{
ListNode *Head;
}List;

void AddNode(List *L, String item)
{
ListNode *currNode, *newNode;
currNode = L->Head;
if (L->Head == NULL)
{
L->Head = (ListNode*)mall oc(sizeof(ListN ode));
Undefined behaviour. Your compiler would have warned you about it, too, if
only you hadn't put in that stupid cast.

Rewrite this as:

L->Head = malloc(sizeof *L->Head);

In general, if p is a pointer to some object type, then:

p = malloc(sizeof *p);
Don't forget to #include <stdlib.h> for the malloc prototype (the absence of
which is what gives you undefined behaviour in the above code).

L->Head->Data = item;
You forgot to check malloc's return value, which could easily have been NULL
(meaning "couldn't allocate the requested memory"). If it was, then
dereferencing that pointer (as you do here) invokes undefined behaviour.
L->Head->Link = NULL;
}
else
{
while(currNode->Link != NULL)
currNode = currNode->Link; //go to the last
newNode = (ListNode*)mall oc(sizeof(ListN ode));
newNode = malloc(sizeof *newNode);

Don't forget to check for NULL.
newNode->Data = item;
newNode->Link = NULL;
currNode->Link = newNode;
}
}

void DeleteNode(List *L, String item)
{
ListNode *currNode, *prevNode;
currNode = L->Head;
prevNode = L->Head;
if (L->Head == NULL)
{
printf("empty list");
}
while(currNode != NULL) //check if first
{
if(strcmp(currN ode->Data.str,item. str)==0) //found
{
if(currNode == L->Head) //first one
{
L->Head == currNode->Link;
currNode->Link = NULL;
free(currNode);
printf("Remove First");
}
else
{
prevNode->Link = currNode->Link;
currNode->Link = NULL;
free(currNode);
printf("removed ");
}
}
else
{
prevNode = currNode;
currNode = currNode->Link; //go to next one
}
}
}
I had a quick look at this, but the complete absence of indentation made the
code very hard to follow, so I gave up.



int main()
{
FILE *fp = NULL;
List list;
String string;
int i;
int records = 0;
char filename[100];
//list.Head = (ListNode*)mall oc(sizeof(ListN ode));
list.Head = NULL;
string.str = (char *)malloc(sizeof (char) *20);
string.str = malloc(20 * sizeof *string.str);

Note: 20 may not be enough. If not, you risk a buffer overrun attack.

Test for NULL.
printf("Enter filename\n");
scanf("%s", filename);
Danger of buffer overrun attack by careless or malicious user.
fp = fopen(filename, "r");

if (fp == NULL)
{ printf ("Error\n");
exit(0);
0 indicates success. On error, I suggest using EXIT_FAILURE (which is
another good reason to #include <stdlib.h>).
}
else
{
fscanf(fp, "%d", &records);
for (i = 0; i<records; i++)
{
fscanf(fp, "%s", string.str);
Buffer overrun may occur.
AddNode(&list, string);
printf("%s ", string.str);
}
printf("\n\nEnt er a Node to delete: ");
Either print a newline here or fflush(stdout);
scanf("%s", string.str);
Buffer overrun may occur.
DeleteNode(&lis t, string);
}
return 1;
return EXIT_SUCCESS or return 0 would be a much better idea. Google the
archives for an explanation.
}

--
Richard Heathfield : bi****@eton.pow ernet.co.uk
"Usenet is a strange place." - Dennis M Ritchie, 29 July 1999.
C FAQ: http://www.eskimo.com/~scs/C-faq/top.html
K&R answers, C books, etc: http://users.powernet.co.uk/eton

[T.M. Sommers]

na1paj wrote:

here's a simple linked list program. the DeleteNode function is
producing an infinit loop i think, but i can't figure out where..
Other posters have already pointed out some errors. Here are more.
#include <stdio.h>
#include <string.h>
#include <stdlib.h>
typedef struct
{
char *str; //str is a dynamic array of characters
int length; //number of characters
} String;

typedef struct node
{
String Data;
struct node* Link;
} ListNode;

typedef struct listStuct
{
ListNode *Head;
}List;

void AddNode(List *L, String item)
{
ListNode *currNode, *newNode;
currNode = L->Head;
if (L->Head == NULL)
{
L->Head = (ListNode*)mall oc(sizeof(ListN ode));
L->Head->Data = item;
L->Head->Link = NULL;
}
else
{
while(currNode->Link != NULL)
currNode = currNode->Link; //go to the last
newNode = (ListNode*)mall oc(sizeof(ListN ode));
newNode->Data = item;
newNode->Link = NULL;
currNode->Link = newNode;
}
}

void DeleteNode(List *L, String item)
{
ListNode *currNode, *prevNode;
currNode = L->Head;
prevNode = L->Head;
if (L->Head == NULL)
{
printf("empty list");
}
while(currNode != NULL) //check if first
{
if(strcmp(currN ode->Data.str,item. str)==0) //found
{
if(currNode == L->Head) //first one
{
L->Head == currNode->Link;
L->Head = currNode->Link;
currNode->Link = NULL;
free(currNode);
printf("Remove First");
}
else
{
prevNode->Link = currNode->Link;
currNode->Link = NULL;
free(currNode);
printf("removed ");
}
break;
}
else
{
prevNode = currNode;
currNode = currNode->Link; //go to next one
}
}
}
int main()
{
FILE *fp = NULL;
List list;
String string;
int i;
int records = 0;
char filename[100];
//list.Head = (ListNode*)mall oc(sizeof(ListN ode));
list.Head = NULL;
string.str = (char *)malloc(sizeof (char) *20);
printf("Enter filename\n");
scanf("%s", filename);
fp = fopen(filename, "r");

if (fp == NULL)
{ printf ("Error\n");
exit(0);
}
else
{
fscanf(fp, "%d", &records);
for (i = 0; i<records; i++)
{
fscanf(fp, "%s", string.str);
AddNode(&list, string);
printf("%s ", string.str);
}
printf("\n\nEnt er a Node to delete: ");
scanf("%s", string.str);
Here is the real problem. Your String structure contains a pointer to
an array of chars. When adding a node, you copy the structure, and
hence the pointer, so every node is pointing to the buffer you
allocated above in main. Including the String structure you are using
as the key to delete.
DeleteNode(&lis t, string);
}
return 1;
}

[goose]

na****@yahoo.co m (na1paj) wrote in message news:<e2******* *************** ****@posting.go ogle.com>...

here's a simple linked list program. the DeleteNode function is
producing an infinit loop i think, but i can't figure out where..
a really good idea would be to turn up the warning/diagnostics
level of your compiler. let your compiler catch the obvious
bugs. I've numbered my "fixes", this lets me only explain
each "fix" once, and refer back to it when I come across it again.

It would be a good idea if you wrote a function that could
loop though a list and print out each element. that way you
can verify that what you added to the list is in the list and
what you deleted from the list has in fact been deleted.


#include <stdio.h>
1. you also need to #include <stdlib.h> for the malloc/free
function calls and string.h (for strncpy):

#include <stdlib.h>
#include <string.h>
typedef struct
{
char *str; //str is a dynamic array of characters
int length; //number of characters
} String;

typedef struct node
{ f struct node
String Data;
struct node* Link;
} ListNode;

typedef struct listStuct
{ f struct listStuct
ListNode *Head;
}List;

void AddNode(List *L, String item)
{ ddNode(List *L, String item)
ListNode *currNode, *newNode; ers
currNode = L->Head; Node; ers
if (L->Head == NULL) Node; ers
{ ULL) Node; ers
L->Head = (ListNode*)mall oc(sizeof(ListN ode)); af***@posting.g oogle.com>...
2. there is no need to cast the return of malloc. yuor compiler
probably complained about this line without the cast because
you didn't include stdlib.h.
3. a better idea is to use sizeof on the variable itself,
not on the type:

L->Head = malloc(sizeof L->Head); e itself,

4. you must *always* check the return of malloc. what in case
there was no memory ? do something like this:

if (L->Head==NULL) {
printf ("no memory for item %s\n", item.str);
return;
}
L->Head->Data = item; .str);
5. there is a fundamental misconception here with regard to the
way you /think/ the String struct works. when you /assign/
item to L->Head->Data, the field "length" gets set correctly.
but the /other/ field ("str") does not get copied into. I
suspect that that is what you would want. you should do this
instead:

L->Head->Data.length = item.length;
L->Head->Data.str = malloc (item.length);
if (L->Head->Data.str==NULL ) {
printf ("no memory for str%s\n", item.str);
free (L->Head);
return;
}
strncpy (L->Head->Data.str, item.str, item.length);
L->Head->Data.str[item.length] = '\0';

L->Head->Link = NULL; ;
} L->Head->Link = NULL; ;
else L->Head->Link = NULL; ;
{ >Link = NULL; ;
while(currNode->Link != NULL)
currNode = currNode->Link; //go to the last af***@posting.g oogle.com>...
newNode = (ListNode*)mall oc(sizeof(ListN ode)); af***@posting.g oogle.com>...
see fix #2, #3 and #4:
newNode = malloc(sizeof *newNode); ode));
af***@posting.g oogle.com>...
if (newNode==NULL) {
printf ("no memory for item %s\n", item.str);
return;
}
newNode->Data = item; .str);
see fix #5:

newNode->Data.length = item.length;
newNode->Data.str = malloc (item.length);
if (newNode->Data.str==NULL ) {
printf ("no memory for str%s\n", item.str);
free (newNode);
return;
}
strncpy (L->Head->Data.str, item.str, item.length);
L->Head->Data.str[item.length] = '\0';
newNode->Link = NULL; ngth);
currNode->Link = newNode; ngth);
}
}

void DeleteNode(List *L, String item)
{ eleteNode(List *L, String item)
ListNode *currNode, *prevNode; ; ngth);
currNode = L->Head;
prevNode = L->Head; vNode; ; ngth);
if (L->Head == NULL)
{ ULL)
printf("empty list"); ; ngth);
6. you have correctly identified this as an empty list.
dont you think it might be better to merely return
immediately ? there is no need to actually check,
because the loop below will never execute if the
list is empty.
} .
while(currNode != NULL) //check if first ode)); af***@posting.g oogle.com>...
{ //check if first ode)); af***@posting.g oogle.com>...
if(strcmp(currN ode->Data.str,item. str)==0) //found sting.google.co m>...
{
if(currNode == L->Head) //first one
{
L->Head == currNode->Link;
7. this line does nothing. your compiler might have emitted
a diagnostic (did your compiler complain?). I replaced it
withs:
L->Head = currNode->Link;
currNode->Link = NULL;
free(currNode);
8. before you can do that, we need to free the memory in item
that I allocated earlier. do this instead:

free (currNode->Data.str);
free (currNode);
printf("Remove First");
}
else
{ item
prevNode->Link = currNode->Link; >...
currNode->Link = NULL;
free(currNode); >...
see #8 above:

free (currNode->Data.str);
free (currNode);
printf("removed ");
} removed ");
}
else
{
prevNode = currNode;
currNode = currNode->Link; //go to next one >...
}
}
}
int main()
{ in()
FILE *fp = NULL; = currNode->Link; //go to next one >...
List list; = currNode->Link; //go to next one >...
String string; = currNode->Link; //go to next one >...
int i; tring; = currNode->Link; //go to next one >...
int records = 0; = currNode->Link; //go to next one >...
char filename[100];
//list.Head = (ListNode*)mall oc(sizeof(ListN ode)); >...
list.Head = NULL; istNode*)malloc (sizeof(ListNod e)); >...
string.str = (char *)malloc(sizeof (char) *20); istNode)); >...
9. sizeof (char) is always equal to one, also see fixes
#2 and #4 above:

string.str = malloc(20); to one, also see fixes
if (string.str==NU LL) {
printf ("no memory\n");
return EXIT_FAILURE;
}
printf("Enter filename\n"); also see fixes
scanf("%s", filename); "); also see fixes
fp = fopen(filename, "r"); also see fixes
fp = fopen(filename, "r"); also see fixes
if (fp == NULL)
{ printf ("Error\n"); >...
exit(0); printf ("Error\n"); >...
}
else
{
fscanf(fp, "%d", &records);
for (i = 0; i<records; i++) "); >...
{
fscanf(fp, "%s", string.str);
10.what in case the string you are trying
to read in is bigger than the space you allocated ?
try this instead:

fscanf(fp, "%19s", string.str);

11.you forget to set the length field of the struct
"string", try this:

string.length = strlen (string.str) +1;
AddNode(&list, string);
printf("%s ", string.str);
}
printf("\n\nEnt er a Node to delete: ");
scanf("%s", string.str);
DeleteNode(&lis t, string);
}
return 1;
}

hth

goose,
hand

[goose]

oops!!! the previous post got messed over somehow, i hope
this gets though without any problems:

na****@yahoo.co m (na1paj) wrote in message news:<e2******* *************** ****@posting.go ogle.com>...

here's a simple linked list program. the DeleteNode function is
producing an infinit loop i think, but i can't figure out where..
a really good idea would be to turn up the warning/diagnostics
level of your compiler. let your compiler catch the obvious
bugs. I've numbered my "fixes", this lets me only explain
each "fix" once, and refer back to it when I come across it again.

It would be a good idea if you wrote a function that could
loop though a list and print out each element. that way you
can verify that what you added to the list is in the list and
what you deleted from the list has in fact been deleted.


#include <stdio.h>
1. you also need to #include <stdlib.h> for the malloc/free
function calls and string.h (for strncpy):

#include <stdlib.h>
#include <string.h>
typedef struct
{
char *str; //str is a dynamic array of characters
int length; //number of characters
} String;

typedef struct node
{
String Data;
struct node* Link;
} ListNode;

typedef struct listStuct
{
ListNode *Head;
}List;

void AddNode(List *L, String item)
{
ListNode *currNode, *newNode;
currNode = L->Head;
if (L->Head == NULL)
{
L->Head = (ListNode*)mall oc(sizeof(ListN ode));
2. there is no need to cast the return of malloc. yuor compiler
probably complained about this line without the cast because
you didn't include stdlib.h.
3. a better idea is to use sizeof on the variable itself,
not on the type:

L->Head = malloc(sizeof L->Head);

4. you must *always* check the return of malloc. what in case
there was no memory ? do something like this:

if (L->Head==NULL) {
printf ("no memory for item %s\n", item.str);
return;
}
L->Head->Data = item;
5. there is a fundamental misconception here with regard to the
way you /think/ the String struct works. when you /assign/
item to L->Head->Data, the field "length" gets set correctly.
but the /other/ field ("str") does not get copied into. I
suspect that that is what you would want. you should do this
instead:

L->Head->Data.length = item.length;
L->Head->Data.str = malloc (item.length);
if (L->Head->Data.str==NULL ) {
printf ("no memory for str%s\n", item.str);
free (L->Head);
return;
}
strncpy (L->Head->Data.str, item.str, item.length);
L->Head->Data.str[item.length] = '\0';

L->Head->Link = NULL;
}
else
{
while(currNode->Link != NULL)
currNode = currNode->Link; //go to the last
newNode = (ListNode*)mall oc(sizeof(ListN ode));
see fix #2, #3 and #4:
newNode = malloc(sizeof *newNode);
if (newNode==NULL) {
printf ("no memory for item %s\n", item.str);
return;
}
newNode->Data = item;
see fix #5:

newNode->Data.length = item.length;
newNode->Data.str = malloc (item.length);
if (newNode->Data.str==NULL ) {
printf ("no memory for str%s\n", item.str);
free (newNode);
return;
}
strncpy (L->Head->Data.str, item.str, item.length);
L->Head->Data.str[item.length] = '\0';
newNode->Link = NULL;
currNode->Link = newNode;
}
}

void DeleteNode(List *L, String item)
{
ListNode *currNode, *prevNode;
currNode = L->Head;
prevNode = L->Head;
if (L->Head == NULL)
{
printf("empty list");
6. you have correctly identified this as an empty list.
dont you think it might be better to merely return
immediately ?
}
while(currNode != NULL) //check if first
{
if(strcmp(currN ode->Data.str,item. str)==0) //found
{
if(currNode == L->Head) //first one
{
L->Head == currNode->Link;
7. this line does nothing. your compiler might have emitted
a diagnostic (did your compiler complain?). I replaced it
withs:
L->Head = currNode->Link;
currNode->Link = NULL;
free(currNode);
8. before you can do that, we need to free the memory in item
that I allocated earlier. do this instead:

free (currNode->Data.str);
free (currNode);
printf("Remove First");
}
else
{
prevNode->Link = currNode->Link;
currNode->Link = NULL;
free(currNode);
see #8 above:

free (currNode->Data.str);
free (currNode);
printf("removed ");
}
}
else
{
prevNode = currNode;
currNode = currNode->Link; //go to next one
}
}
}
int main()
{
FILE *fp = NULL;
List list;
String string;
int i;
int records = 0;
char filename[100];
//list.Head = (ListNode*)mall oc(sizeof(ListN ode));
list.Head = NULL;
string.str = (char *)malloc(sizeof (char) *20);
9. sizeof (char) is always equal to one, also see fixes
#2 and #4 above:

string.str = malloc(20);
if (string.str==NU LL) {
printf ("no memory\n");
return EXIT_FAILURE;
}
printf("Enter filename\n");
scanf("%s", filename);
fp = fopen(filename, "r");

if (fp == NULL)
{ printf ("Error\n");
exit(0);
}
else
{
fscanf(fp, "%d", &records);
for (i = 0; i<records; i++)
{
fscanf(fp, "%s", string.str);
10.what in case the string you are trying
to read in is bigger than the space you allocated ?
try this instead:

fscanf(fp, "%19s", string.str);

11.you forget to set the length field of the struct
"string", try this:

string.length = strlen (string.str) +1;
AddNode(&list, string);
printf("%s ", string.str);
}
printf("\n\nEnt er a Node to delete: ");
scanf("%s", string.str);
DeleteNode(&lis t, string);
}
return 1;
}

hth

goose,
hand

[Rahul Agarkar]

na****@yahoo.co m (na1paj) wrote in message news:<e2******* *************** ****@posting.go ogle.com>...
Hi,

The problem is because of currNode in the function DeleteNode. When
you say free(currNode), it just frees the memory allocated to that
variable but doesn't clear the contents of the memory.

In short, that memory area is marked as free that can be reallocated
to some other variable but the contents still remain the same.

This is a very common problem related with the common programming
practice.

For clearing the memory, just don't rely on free(). The good
programming practice is to assign NULL to the variable after calling
free().

Please make some changes in the code. The changes are given below:

here's a simple linked list program. the DeleteNode function is
producing an infinit loop i think, but i can't figure out where..
#include <stdio.h>
typedef struct
{
char *str; //str is a dynamic array of characters
int length; //number of characters
} String;

typedef struct node
{
String Data;
struct node* Link;
} ListNode;

typedef struct listStuct
{
ListNode *Head;
}List;

void AddNode(List *L, String item)
{
ListNode *currNode, *newNode;
currNode = L->Head;
if (L->Head == NULL)
{
L->Head = (ListNode*)mall oc(sizeof(ListN ode));
L->Head->Data = item;
L->Head->Link = NULL;
}
else
{
while(currNode->Link != NULL)
currNode = currNode->Link; //go to the last
newNode = (ListNode*)mall oc(sizeof(ListN ode));
newNode->Data = item;
newNode->Link = NULL;
currNode->Link = newNode;
}
}

void DeleteNode(List *L, String item)
{
ListNode *currNode, *prevNode;
currNode = L->Head;
prevNode = L->Head;
if (L->Head == NULL)
{
printf("empty list");
}
while(currNode != NULL) //check if first
{
if(strcmp(currN ode->Data.str,item. str)==0) //found
{
if(currNode == L->Head) //first one
{
L->Head == currNode->Link; -----> This code is of no use as '==' is a comparison operator whereas
here you have to use a assignment operator '='. Hence change the above
line to :
L->Head = currNode->Link;
currNode->Link = NULL;
free(currNode); -----> Add the following code to stop your programme to go into the
infinite loop.
currNode = NULL; printf("Remove First");
}
else
{
prevNode->Link = currNode->Link;
currNode->Link = NULL;
free(currNode); -----> Add the following code to stop your programme to go into the
infinite loop.
currNode = NULL; printf("removed ");
}
}
else
{
prevNode = currNode;
currNode = currNode->Link; //go to next one
}
}
}
int main()
{
FILE *fp = NULL;
List list;
String string;
int i;
int records = 0;
char filename[100];
//list.Head = (ListNode*)mall oc(sizeof(ListN ode));
list.Head = NULL;
string.str = (char *)malloc(sizeof (char) *20);
printf("Enter filename\n");
scanf("%s", filename);
fp = fopen(filename, "r");

if (fp == NULL)
{ printf ("Error\n");
exit(0);
}
else
{
fscanf(fp, "%d", &records);
for (i = 0; i<records; i++)
{
fscanf(fp, "%s", string.str);
AddNode(&list, string);
printf("%s ", string.str);
}
printf("\n\nEnt er a Node to delete: ");
scanf("%s", string.str);
DeleteNode(&lis t, string);
}
return 1;
}

The above changes will interrupt the infinite loop. When some memory
is freed using free() call, assigning NULL to that variable will clear
up that memory area.

These things are never explained in any of the books. One can only
learn these things with experience.

[nobody]

"Rahul Agarkar" <ra***********@ hotmail.com> wrote in message
news:24******** *************** **@posting.goog le.com...

na****@yahoo.co m (na1paj) wrote in message news:<e2******* *************** ****@posting.go ogle.com>... Hi,

The problem is because of currNode in the function DeleteNode. When
you say free(currNode), it just frees the memory allocated to that
variable but doesn't clear the contents of the memory.
This is not a problem., as far as I can see it.
In short, that memory area is marked as free that can be reallocated
to some other variable but the contents still remain the same.
So what? Contents was random even after initial call to malloc().
This is a very common problem related with the common programming
practice.
Maybe it's common, but yet again, it's not a problem, generally speaking.
For clearing the memory, just don't rely on free(). The good
programming practice is to assign NULL to the variable after calling
free().
This doesn't "clear" memory, whatever you mean by "clear". It just
prevents UB(?) if an attempt is made to free same block more than once.

[snip]
The above changes will interrupt the infinite loop. When some memory
is freed using free() call, assigning NULL to that variable will clear
up that memory area.
It will not (regardless of definition of "clear"), see above.
These things are never explained in any of the books. One can only
learn these things with experience.

Obviously, there is good reason not to put nonsense in the books.
Hope you are not going to write any book on C any time soon.

[Kevin Goodsell]

nobody wrote:

"Rahul Agarkar" <ra***********@ hotmail.com> wrote in message
news:24******** *************** **@posting.goog le.com...

For clearing the memory, just don't rely on free(). The good
programming practice is to assign NULL to the variable after calling
free().

This doesn't "clear" memory, whatever you mean by "clear". It just
prevents UB(?) if an attempt is made to free same block more than once.

It doesn't even do that. All it does is prevent UB if an attempt is made
to free the same block more than once *through the same pointer*. I
think errors due to freeing the same block more than once are usually
the result of freeing it through multiple pointers to the same location.
Setting your pointers to NULL after freeing them doesn't help matters.
In fact, it may make matters worse since people tend to assume that if
they assign NULL to freed pointers, non-NULL pointers are safe to free.
Clearly this is not true in general.

-Kevin
--
My email address is valid, but changes periodically.
To contact me please use the address from a recent posting.

[nobody]

"Kevin Goodsell" <us************ *********@never box.com> wrote in message
news:vH******** *********@newsr ead3.news.pas.e arthlink.net...

nobody wrote:

"Rahul Agarkar" <ra***********@ hotmail.com> wrote in message
news:24******** *************** **@posting.goog le.com...

For clearing the memory, just don't rely on free(). The good
programming practice is to assign NULL to the variable after calling
free().

This doesn't "clear" memory, whatever you mean by "clear". It just
prevents UB(?) if an attempt is made to free same block more than once.

It doesn't even do that. All it does is prevent UB if an attempt is made
to free the same block more than once *through the same pointer*. I

Sorry for insufficient clarity on my part. This is what meant, in the
context
of (snipped) code.
think errors due to freeing the same block more than once are usually
the result of freeing it through multiple pointers to the same location.
Setting your pointers to NULL after freeing them doesn't help matters.
In fact, it may make matters worse since people tend to assume that if
they assign NULL to freed pointers, non-NULL pointers are safe to free.
Clearly this is not true in general.

Right. Again, I was referring (perhaps insufficiently) to code like OP's,
where
it's always "same" pointer, but it's difficult to trace multiple free()
calls on same
pointer. (Personally, I prefer *not* to assign NULL /I even prefer more not
to assign 0/, so problem is not hidden but is exhibited via UB and fixed at
the first opportunity.)
BTW, If I didn't read the reply (to which I had replied) to OP, I would
polemize
with your statement "people tend to assume that if ...". Obviously, some
people do (tend to assume illogical things).