Help with my linked list...

[ronaldh12]

I'm working on the insert function for a linked list. I can't figure out what's wrong with it. I'm trying to create a phonebook. Here's what I have...it just crashes on me when the function is called in main. I know that it's this function because of some debugging, but can't figure out what the problem is because no warnings are popping up. Well, here's the code.

void insert(Entryptr newnodeptr)
{
    Entryptr trailing = head, target = head;
    if (head->next == NULL) //List is empty
        head->next = newnodeptr;
    else if (strcmp(newnodeptr->lastname, head->lastname)>0)
    {


        newnodeptr->next = head;
        head = newnodeptr;
    }
    else
    {
        target = trailing->next;
        while(target != NULL)
        {
            if (strcmp(newnodeptr->lastname, target->lastname)>0)
            {
                newnodeptr->next = target;
                trailing->next = newnodeptr;
            }
            else if (strcmp(newnodeptr->lastname, target->lastname)==0)
            {
                if (strcmp(newnodeptr->firstname, target->firstname)>0)
                {
                    newnodeptr->next = target;
                    trailing->next =  newnodeptr;
                }
                else
                {
                    trailing = target;
                    target = trailing->next;
                }
            }
            else
            {
                trailing = target;
                target = trailing->next;
            }
        }


    }


}

[MK27]

This is going to be hard to diagnose without the definition of your types, eg, "Entryptr".

BTW, defining pointer types because you don't like typing * is a bad practice normally; it is a form of obfuscation.

Obfuscated code - Wikipedia, the free encyclopedia

At a glance, I'd say without a break after you insert the node, your while() loop is going to infinitely repeat the insertion -- although that would not cause a crash, just a racing wheel.

[ronaldh12]

I see. I shall post my struct and typedefs.

struct entry{
    char firstname[20];
    char lastname[20];
    char number[11];
    struct entry *next;
};
typedef struct entry Entry;
typedef Entry *Entryptr;

I'm basing this largely off of some of the code that my professor gave me, and he has said specifically that he hasn't programmed in C in years (great professor I tell ya' >.>). So, I'm not entirely sure what you mean by what you said. I don't see too much wrong with the pointer declarations. Maybe you can give an example of what you mean? Thanks. :]

[ronaldh12]

I just realized that I need to change the array size of number to make it bigger, but that isn't something anybody would catch without seeing the rest of the code *goes back to coding*.

[MK27]

Well, if your professor has told you to do it (typedef pointers) then you don't really have a choice, but IMO it intentionally removes an element of information when reading the code, which is obfuscation. By analogy, why give types and variables descriptive names? Why not just call them "whatever_t"?:

whatever bob;

Now we know "bob" is of type "whatever". Nice and clear (not). So the point about disguising pointer types is that * means something. Why intentionally reduce the clarity and meaninfulness of your code??

Speaking of, I guess I did not spell this out clearly enough before, WRT your while() being infinite.

line 25

 
                if (strcmp(newnodeptr->firstname, target->firstname)>0)
                {
                    newnodeptr->next = target;
                    trailing->next =  newnodeptr;
                }

This is where you insert the node. But because you do not break out of the while at that point, guess what? Target is still the same node, newnodeptr is still the same node, trailing is still the same node, target is still != NULL, and this if() block will repeat ad infinitum.

I presume by "crash" you mean segmentation fault? If so, just run the code in a debugger and find the problem.