Creating a singly linked list in C

Question

I'm trying to create a singly linked list from an input text file for an assignment. I'm trying to do it a little bit at a time so I know my code is not complete. I tried creating the head pointer and just printing out its value and I can't even get that to work, but I'm not sure why. I included the struct, my create list, and print list functions. I didn't include the open file since that part works.

typedef struct List
{
   struct List *next;   /* pointer to the next list node */
   char *str;           /* pointer to the string represented */
   int count;           /* # of occurrences of this string */
} LIST;

LIST *CreateList(FILE *fp) 
{
    char input[LINE_LEN];
    LIST *root;             /* contains root of list             */
    size_t strSize;         
    LIST *newList;          /* used to allocate new list members */

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

        strSize = strlen(input) + 1;

        /* create root node if no current root node */
        if (root == NULL) {
            if ((newList = (LIST *)malloc(sizeof(LIST))) == NULL) {
                printf("Out of memory...");
                exit(EXIT_FAILURE);
            } 
            if ((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;
        }
    }
        return root;
}

/* Prints sinly linked list and returns head pointer */
LIST *PrintList(const LIST *head) 
{
    int count;

    for (count = 1; head != NULL; head = head->next, head++) {
        printf("%s    %d", head->str, head->count);
    }                       
    return head;     /* does this actually return the start of head ptr, b/c I want to 
                            return the start of the head ptr. */
}

You do not want to have head++ in PrintList, head = head->next already increments the pointer. — Lukáš Lalinský
– Lukáš Lalinský, Commented Feb 22, 2010 at 7:18
did you ask this twice..? stackoverflow.com/questions/2309618/single-linked-lists-in-c — lorenzog
– lorenzog, Commented Feb 22, 2010 at 8:59

wallyk · Accepted Answer · 2010-02-22 07:16:50Z

2

root has an undefined value, so it won't initialize. The second line of CreateList should be

LIST *root = NULL;

Also, further down there is allocation apparently for the details of the item, but a) the code fails to capture the allocation and save it anywhere, and b) the size of the allocation should be strSize, not the length of the variable itself. There are several ways to fix it, but the most straightforward would be:

newList->str = (char *)malloc(strSize);
if (newList->str == NULL)

answered Feb 22, 2010 at 7:16

wallyk

58.1k17 gold badges92 silver badges155 bronze badges

Sign up to request clarification or add additional context in comments.

Comments

Patrick · Accepted Answer · 2010-02-22 07:40:23Z

1

The second malloc allocates memory but its return value is not assigned to anything, so that allocated memory is lost.

newList is allocated but not initialized, so using a memcpy to copy memory to newList->str will fail since newList->str points to nothing. Probably you wanted the result of the second malloc to be assigned to newList->str, but you forgot it.

answered Feb 22, 2010 at 7:40

Patrick

23.7k13 gold badges74 silver badges134 bronze badges

Comments

David Kanarek · Accepted Answer · 2010-02-22 07:58:13Z

1

You shouldn't be incrementing head after head = head->next in the for loop. PrintList will return NULL every time since the loop wont stop until head is NULL. Why do you need to return the head of the list you just passed to the function anyway?

Edit:

LIST *current = head;
while (current != NULL) {
    printf("%s    %d", current->str, current->count);
    current = current->next;
}

edited Feb 22, 2010 at 7:58

answered Feb 22, 2010 at 7:18

David Kanarek

12.6k5 gold badges49 silver badges62 bronze badges

2 Comments

Crystal Over a year ago

Just what my professor wants. And the const char * parameter confused me on how I would iterate through the loop.

David Kanarek Over a year ago

You should create a pointer to the current node and initialize it with head. Then don't touch head again until you return it.

Collectives™ on Stack Overflow

Creating a singly linked list in C

3 Answers 3

Comments

Comments

2 Comments

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

Comments

Comments

2 Comments

Your Answer

Sign up or log in

Post as a guest

Linked

Related