Linked List implementation

I'm trying to implement a list interface in C. It is a sorted list with items declared as

Code:

---

typedef struct {
        char* str;
        int len;
} Item;

---

I think the problem is in this function. I can create the list from a data file containing strings and then print it out using another function. However, when printing the list it prints out the same string twice. I will attach all relevant files.

Code:

---

void FindLocation(List* L, ListNode** Prev, Item item)
{
        ListNode* Iterator;
        Item anItem;
       
        Iterator = L->head;
        *Prev = NULL;

        while( Iterator)
        {
                *Prev = Iterator;
                if(strcmp(item.str, Iterator->data.str) > 0 )
                {
                        fprintf(stdout, "The new item is greater than the iterator with data\n%s\n", Iterator->data.str);
                        break;
                }
                Iterator = Iterator->next;
        }
}

---

The output never contains the debug string in the if block in the above function.

Here is a sample output:

Code:

---

t3\LinkSort\Debug>LinkSort input.data
Previous node is null.
New node has data This is line one.
.
Head node has data This is line one.
.
Previous node has data THis is line two..
New node has data THis is line two..
THis is line two.
THis is line two.

---

Code:

---

temp.str = (char*)malloc(sizeof(line));

---

Don't cast malloc. And free it at the end of your program, too.

You don't close the file fp.

Code:

---

item->len = (int)strlen(item->str);

---

I don't think you should cast strlen(), either.

is there any reason why you're taking ListNode** Prev and then setting it to NULL!? Clean up your code a bit, I see alot of things that could be left out. Such as Item anItem; that's not being used. Also, are you sure Iterator isn't NULL, why don't you check that. One thing you need to learn as a programmer is to check EVERYTHING again and again. Try printing each string in the while statement to the console for a debug test.

Quote:

---

Originally Posted by dwks

Code:

---

temp.str = (char*)malloc(sizeof(line));

---

Don't cast malloc. And free it at the end of your program, too.

You don't close the file fp.

Code:

---

item->len = (int)strlen(item->str);

---

I don't think you should cast strlen(), either.

---

Thank you for your advice, even though it may be unrelated to the question.

The code is in no way complete as it stands but I appreciate your tips.

Quote:

---

Originally Posted by durban

is there any reason why you're taking ListNode** Prev and then setting it to NULL!?

---

Yes because if the previous element is the head of the list, then I will need to add it to the beginning, and I need a way to signal that to the calling function. Thus in the calling method we have:

Code:

---

if(!Prev)
//make the new element the head of the list

---

Quote:

---

Clean up your code a bit, I see alot of things that could be left out.

---

Yes I know. I am not finished with it. Anything that seems out of place was at one time for debugging. Once everything works, they will be removed. Thanks for the advice.

Quote:

---

Also, are you sure Iterator isn't NULL, why don't you check that.

---

if Iterator is NULL, while(Iterator) become while(NULL) which is while(0) which means the block doesn't execute.

Then the function that has set *Prev to NULL will return control to the calling function where if the pointer Prev is NULL then we set the current node we are adding to be the head node of the list.

Quote:

---

One thing you need to learn as a programmer is to check EVERYTHING again and again.

---

Wait, so testing our logic and implementation is in the description of a programmer? :D