Scanf skips second line of input, always

Question

I'm trying to read lines of text data, and store them in a linked list. Input data looks something like this:

David Copperfield
Charles Dickens
4250
24.95
32.95
10
6
END_DATA

The way I read the data is to first read the first line to see if it is END_DATA. If it's not, then I pass the first line into a function which creates a linked list Node with the book data inside. For some reason, scanf does not read the second line after I pass the first line to the function.

When I try to print the Nodes after they've read the data, my output looks like this.

David Copperfield 

0 
0.000000 
0.000000 
0 
0
Charles Dickens 

4250 
24.950001 
32.950001 
10 
6
Charles Dickens 

0 
0.000000 
0.000000 
0 
0

Source Code is below

#include "lab3p1.h"
#include <stdio.h>
#include <stdlib.h>
#include <string.h>


struct NodeRec {
    Book data;
    struct NodeRec *next;
};

typedef struct NodeRec Node;

float findRevenue(Node *head);
float totalWholesaleCost(Node *head);
float totalProfit(Node *head);
float totalBooksSold(Node *head);
float averageProfitPerSale(Node *head);
void printNodes(Node *head);

void insertNode(Node* head, Node* newNode);

Node* createNewNode(char titleS[40]) {
    Node* newNode;
    newNode = malloc(sizeof(Node));
    if (newNode == NULL){
        printf("ERROR ALLOCATING SPACE");
        exit(-1);
    }
    else {
        strcpy(newNode->data.title, titleS);
        scanf("%40[^\n]*c", (char *)&newNode->data.author);

        scanf ("%i %f %f %i %i", &newNode->data.number, &newNode->data.wholesaleprice, &newNode->data.retailprice, &newNode->data.wholesalequantity, &newNode->data.retailquantity);

        printf ("%s \n%s \n%i \n%f \n%f \n%i \n%i\n", newNode->data.title, newNode->data.author,newNode->data.number, newNode->data.wholesaleprice, newNode->data.retailprice, newNode->data.wholesalequantity, newNode->data.retailquantity);

    }

    return newNode;
}


void readBooks(Node* head) {

    char firstline[40];
    char enddataString[40] = "END_DATA";
    scanf("%40[^\n]*c", (char *)&firstline);


    while (strcmp(firstline, enddataString)) {

        Node* newNode = createNewNode(firstline);
        insertNode(head, newNode);

        scanf("%40[^\n]*c", (char *)&firstline);
    }
}



void insertNode(Node* head, Node* newNode) {

    if (head == NULL) {
        head = newNode;
    }
    else {
        Node *preNode, *currentNode;

        int n = newNode->data.number;
        currentNode = head;

        while (currentNode != NULL && (currentNode->data.number) > n ) {
            preNode = currentNode;
            currentNode = currentNode->next;

        }
        newNode->next = currentNode;
        preNode->next = newNode;
    }
}

Answer 1

The problem is with your scanf format string. In the following line there are a few problems:

scanf("%40[^\n]*c", (char *)&firstline);

First, you wish to read everything up to the end of the line (40 characters or less). %40[^\n] will do that. But there is no need for the c .. the bracketed expression takes the place of it.. so, scanf is probably just looking for an ordinary character 'c' in the input.. not what you want, and it will cause problems for you. The asterisk also has no meaning that I know of in that position.

The second problem is that since you were telling scanf to only consume characters up to the newline, the newline remains in the stdin buffer waiting to be consumed. Since the next scanf also refuses to consume a newline, it will consume nothing at all. When you use scanf you must design your format strings to either assign or discard EVERY character in the input or your parsing will fail.

By replacing this with:

scanf("%40[^\n] ", (char *)&firstline);

..your program will work. This should also be used in your author scanf call. The format string here basically says "read everything that is not a newline.. until you see either a newline character or get 40 characters. Then skip any whitespace you see". This will consume stdin up to the first character in the next line, which is what you want.

But basically scanf is a bear and hard to use correctly which is why it is rarely used for this kind of thing anymore. I never use it. For example have you considered what will happen in your program if an author or title contains MORE than 40 characters? The man page for scanf says

Reading of characters stops either when this maximum is reached or when a nonmatching character is found, whichever happens first.

This means that the unread characters will remain in the buffer and foul up the parsing of all your remaining lines.

Sure it is possible to design a scanf format that would handle it.. you would have to set it up to consume everything other than a newline and then throw it away.. and then leave the last space to consume the newline. I can't get that part working right now.. but you should :)

The other major problem you have is with your null terminators. First, every string must be null terminated, and the man page for the scanf c format specifier says

..the next pointer must be a pointer to char, and there must be enough room for all the characters (no terminating null byte is added)

So if you are telling scanf to read 40 characters, your pointer to storage had better point to at least 41! Eg:

char author[41];

And null terminating is your problem! Which you are not doing.. and which will lead to a crash. You are just getting really lucky right now.

One way to quickly get out of the null terminator issue is to use calloc() to allocate your node, instead of malloc(). With calloc() your memory is promised to be filled with all null bytes from the get go...

Anyway, I think you have got enough to go on for now. If I were you I would explore using fgets() to read each line individually from the input, and then various parsing functions to get your values.. eg atoi to parse integers. It takes a bit more work but is more reliable and easy to understand in the end. If you stick with scanf you must really read that man page and try to visualize how each character in your input will be consumed or not consumed. Also, make sure that each string you store is properly null terminated. In c these things are not done for you, you have to think about them.

Answer 2

The main issue is that a leftover '\n' from previous input "%i %f %f %i %i" is left in stdin. When scanf("%40[^\n]... is executed, nothing is save and '\n' remains in stdin.

Moral of the story:

1. Always check the result from I/O functions like fopen, fseek, fscanf, scanf, fgets, fgetc, ...

2. Never use scanf()

Use fgets() or getline() to get a line of data. Then parse it, checking for unexpected results as you go. A bit more code, but less time on SO.