Single linked lists in C

Question

I'm basically trying to create a linked list from a text file and add a new member every time the words are different, and increment the count if the words are the same (hw assignment). I thought I did it correctly, but it seems to add a member no matter what. I'm wondering if I'm traversing the list incorrectly while I search? Here is my code. Any thoughts? Thanks!

LIST *CreateList(FILE *fp) 
{
    char input[LINE_LEN];
    LIST *root= NULL;             /* contains root of list             */
    size_t strSize;    
    LIST *newList;          /* used to allocate new list members */
    int same;             /* if string is same */

    while (fscanf(fp, BUFFMT"s", input) != EOF) {

        strSize = strlen(input) + 1;

            if (root == NULL) {
                if ((newList = (LIST *)malloc(sizeof(LIST))) == NULL) {
                    printf("Out of memory...");
                    exit(EXIT_FAILURE);
                } 
                if ((newList->str = (char *)malloc(sizeof(strSize))) == NULL) {
                    printf("Not enough memory for %s", input);
                    exit(EXIT_FAILURE);
                }
                memcpy(newList->str, input, strSize);   /*copy string    */
                newList->count = START_COUNT;
                newList->next = NULL;
                root = newList;
            }
            /* if not root node, add node, or increment count */
            else {
                same = ListSame(newList, input);
                if (same == 1) {
                    root->count++;
                }
                else {
                    if ((newList = (LIST *)malloc(sizeof(LIST))) == NULL) {
                        printf("Out of memory...");
                        exit(EXIT_FAILURE);
                    } 
                    if ((newList->str = (char *)malloc(sizeof(strSize))) == NULL) {
                        printf("Not enough memory for %s", input);
                        exit(EXIT_FAILURE);
                    }
                    memcpy(newList->str, input, strSize);   /*copy string    */
                    newList->count = START_COUNT;
                    newList->next = root->next;
                    root->next = newList;
                }
            }
    }
        return root;
}

int ListSame(LIST *head, char *input) 
{
    LIST *start = head;
    for (; start != NULL; start = start->next) {
        if (strcmp(head->str, input) == 0) {
            return 1;   
        }
    }
    return 0;
}

your code would be easier to follow, if you used 'seperation of concerns'; i.e. extract the list code from the file parsing code... — Mitch Wheat, Feb 22 '10 at 08:23
Your best bet is to learn how to debug. Assuming you're using GCC, you can use gdb to debug: http://cs.baylor.edu/~donahoo/tools/gdb/tutorial.html, http://www.google.com/search?q=how+to+debug+gdb. — outis, Feb 22 '10 at 08:26
did you ask this twice or are you trying to implement a link-edlist of question on stack overflw as well? :) http://stackoverflow.com/questions/2309359/creating-a-singly-linked-list-in-c — lorenzog, Feb 22 '10 at 08:58
Sorry, questions were related, but not the same question :). — Crystal, Feb 22 '10 at 16:25

Eric Warmenhoven · Accepted Answer · 2010-02-22T08:34:47.947

You're calling ListSame with newList, which will always only have the last created node. It looks like you want to pass root to ListSame.

Also, in ListSame, it's always checking head->str, not start->str, which is your looping variable.

Also, if ListSame returns true (1), you increment root->count; I'm not sure if you want to increment the root node's count (count how many duplicates total) or the count for the node that has the duplicate (count how many times each word shows up). If it's the latter, either ListSame is going to need to return which node is the duplicate, or it's going to need to increment the count itself.

Also, the way you're building the list is a little confusing. When you say:

newList->next = root->next;
root->next = newList;

root->next before this will either be NULL, or the last created node. So when you do the insert here, you're always inserting it as the second item in the list. That's probably not what you want. If you want to append, you should keep track of the tail as well as the head; if you want to prepend, just set newList->next = root; root = newList;

score 0 · Answer 2 · answered Feb 22 '10 at 08:28

0

LIST *start = head;
for (; start != NULL; start = start->next) {
    if (strcmp(head->str, input) == 0) {
        return 1;   
    }
}

should you be using

strcmp(start->str

answered Feb 22 '10 at 08:28

djna

54,992
14
74
117

Single linked lists in C

2 Answers2

Linked