views:

603

answers:

3

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. */
}
+2  A: 

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)
wallyk
+1  A: 

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;
}
David Kanarek
Just what my professor wants. And the const char * parameter confused me on how I would iterate through the loop.
Crystal
You should create a pointer to the current node and initialize it with head. Then don't touch head again until you return it.
David Kanarek
+1  A: 

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.

Patrick