Trouble with a loop condition - end of file

Question

Current code iteration:

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <ctype.h>

#define MAX_LINE_SIZE 60
#define MAX_LIST_SIZE 15
#define MAX_QUIZ_SIZE 10

typedef struct question {
    char *question;
    char **choices;
    int n_choices;
    char *correct_answer;
} QUESTION;

char *dupString(const char *s) {
    // copies a string
    // CHECK TO SEE IF DUP WORKS

    char *dup = malloc(strlen(s) + 1);
    if (strcpy(dup, s)) {
        return dup;
    } else {
        printf("Error duplicating string");
        return 0;
    }
}

void free_question(QUESTION *q) {
    // free memory
    for(int i = 0; i < q->n_choices; i++) {
        free(q->choices[i]);
    }
    free(q->choices);
    free(q->question);
    free(q->correct_answer);

    // set pointers to null
    for(int i = 0; i < q->n_choices; i++) {
       q->choices[i] = NULL;
    }
    q->choices = NULL;
    q->question = NULL;
    q->correct_answer = NULL;
}

static int read_info(FILE *pData, char **ptr)
{
    char line[MAX_LINE_SIZE];
    if (fgets(line, sizeof(line), pData) == 0 ||
        (*ptr = dupString(line)) == 0)
        return EOF;
    return 0;
}

static int read_number(FILE *pData, int *num, QUESTION *q)
{
    char line[MAX_LINE_SIZE];
    if (fgets(line, sizeof(line), pData) == 0 ||
        (*num = atoi(line)) < 2 || q->n_choices > 9)
        return EOF;
    return 0;
}

static int read_choices(FILE *pData, QUESTION *q)
{
    char line[MAX_LINE_SIZE];
    for (int i = 0; i < q->n_choices; i++)
    {
        if (fgets(line, sizeof(line), pData) == 0 ||
            (q->choices[i] = dupString(line)) == 0)
            return EOF;
    }
    return 0;
}

int parseQuestion(FILE *pData, int qnum, QUESTION *q)
{
    *q = (QUESTION){ NULL, NULL, 0, NULL };

    if (read_info(pData, &q->question) == EOF ||
        read_number(pData, &q->n_choices, q) == EOF ||
        (q->choices = calloc(q->n_choices, sizeof(char *))) == 0 ||
        read_choices(pData, q) == EOF ||
        read_info(pData, &q->correct_answer) == EOF)
    {
        
        return EOF;
    }

    return 0;
}

struct question makeQuestion(FILE *pData, QUESTION *q) {

    int qIndex, numChoices; 
    char question[MAX_LINE_SIZE], temp[MAX_LINE_SIZE], choices[MAX_LINE_SIZE], correctAns[MAX_LINE_SIZE];

    // Eat first line = QUESTION
    fgets(question, MAX_LINE_SIZE, pData);
    q->question = dupString(question);
    

    // Eat second line = NUMBER OF CHOICES
    fgets(temp, MAX_LINE_SIZE, pData);
    numChoices = atoi(temp);
    q->n_choices = numChoices;

    // Allocate memory
    q->choices = calloc(q->n_choices, sizeof(char*));

    // Eat nth lines = CHOICES
    for (qIndex=0; qIndex<=numChoices-1; qIndex++) {
        fgets(choices, MAX_LINE_SIZE, pData);
        q->choices[qIndex] = dupString(choices);
    }
    
    // Eat nth + 1 line = CORRECT ANSWER
    fgets(correctAns, MAX_LINE_SIZE, pData);
    q->correct_answer = dupString(correctAns);

    return *q;  
}           

int ask(QUESTION *q) {
    // Return 1 for correct guess, 0 for incorrect guess.
    
    int choice;

    printf("\n%s\n", q->question);

    for (int i = 0; i <= q->n_choices-1; i++) {
        printf("%d : %s", i+1, q->choices[i]);
    }

    do {
        printf("Select an answer [1-%d]: ", q->n_choices);
        scanf("%d", &choice);
        
        if (choice == 0) {
            return 2;
        }

        /* Check guess */
        int ret;
        size_t size = sizeof(q->correct_answer);
        ret = memcmp(q->choices[choice-1], q->correct_answer, size);
    
        if (ret == 0) {
            return 1;
        }
    } while (choice < 1 || choice > q->n_choices);

    return 0;
}


int main() {

    int num = 0;        // question being asked
    int score = 0;  // incorrect guesses
    char temp[MAX_LINE_SIZE]; // temp for loop condition

    FILE* pData;

    char *filename = "tickle.txt";
    char c;

    if ((pData = fopen(filename, "r"))) {

        printf("Welcome to the 2014 Quiz-festival!\n\n");
        printf("Are you ready to begin? [Y/y]:  ");
        c = getchar();

        if (c == 'Y' || c == 'y') {

            QUESTION q;
            while (parseQuestion(pData, ++num, &q) == 0) {
                makeQuestion(pData, &q);
                
                if (ask(&q) == 1) {
                    ++score;
                    printf("Correct!\n");
                } else {
                    printf("Oops, bad luck!\n");
                }
                free_choices(&q);
            }       

        } else {
            printf("Come back again.\n");
            return 0;
        }

    } else {
        printf("File failed to open.");
        return 0;
    }

    fclose(pData);
    return 0;
}

It seems to be working, however, it starts at the second quiz. Now I'm just totally confused.

Example data file:

Question 1

3 (number of answers)

Answer 1a

Answer 1b

Answer 1c

Answer 1a (this is the correct answer)

Question 2

2

Answer 2a

Answer 2b

Answer 2b (this is the correct answer)

The program starts at Question 2. Is there something going on with the memory? I can't seem to wrap my head around it.

Answer 1

Since the parseQuestion() function is the one that would detect the EOF, it should return a value that indicates whether there's a question to process (assuming EOF if there isn't).

Given that's the case, I'd probably rewrite its interface quite a bit:

QUESTION question;
while (parseQuestion(pData, ++num, &question) == 0)
{
    …process a known-to-be-valid question…
}

I'd let it do the prompt, passing the file pointer, the question number, and a pointer to a question structure which it would fill in. It would return 0 on success and some other value (perhaps EOF) on failure.

This is more or less the standard way to deal with such issues in C. Taking a step back, that's how you check that fgets() works:

char line[4096];
while (fgets(line, sizeof(line), fp) != 0)
{
    …process line…
}

---

You might rewrite parseQuestion() like this:

int parseQuestion(FILE *pData, int qnum, QUESTION *q)
{
    char line[MAX_LINE_SIZE];

    *q = (QUESTION){ NULL, NULL, 0, NULL };

    printf("******** Question: %d ********\n", num);

    if (fgets(line, sizeof(line), pData) == 0 ||
        (q->question = dupString(line)) == 0)
        return EOF;

    if (fgets(line, sizeof(line), pData) == 0)
    {
        free_choices(q);
        return EOF;
    }
    q->n_choices = atoi(line);
    if (q->n_choices < 2 || q->n_choices > 9)
    {
        fprintf(stderr, "Invalid number of choices %d (should be 2..9)\n",
                q->n_choices);
        free_choices(q);
        return EOF;
    }

    q->choices = calloc(q->n_choices, sizeof(char *));
    if (q->choices == 0)
    {
        fprintf(stderr, "Memory allocation failed)\n");
        free_choices(q);
        return EOF;
    }

    for (int i = 0; i < q->n_choices; i++)
    {
        if (fgets(line, sizeof(line), pData) == 0 ||
            (q->choices[i] = dupString(line)) == 0)
        {
            free_choices(q);
            return EOF;
        }
    }

    if (fgets(line, sizeof(line), pData) == 0 ||
        (q->correct_answer = dupString(line)) == 0)
    {
        free_choices(q);
        return EOF;
    }

    return 0;
}

However, there's a lot of repetition in that code in the error handling; you might prefer to try and get rid of some of it. You could call free_choices() before the first return EOF;, but it would have nothing to do (but the marginal performance overhead doesn't matter when you're dealing with an error). Using calloc() to allocate the array of pointers is important; it means you can safely call free_choices() when the array is partially filled. Such observations lead to:

static int read_info(FILE *pData, char **ptr)
{
    char line[MAX_LINE_SIZE];
    if (fgets(line, sizeof(line), pData) == 0 ||
        (*ptr = dupString(line)) == 0)
        return EOF;
    return 0;
}

static int read_number(FILE *pData, int *num)
{
    char line[MAX_LINE_SIZE];
    if (fgets(line, sizeof(line), pData) == 0 ||
        (*num = atoi(line)) < 2 || q->n_choices > 9)
        return EOF;
    return 0;
}

static int read_choices(FILE *pdata, QUESTION *q)
{
    for (int i = 0; i < q->n_choices; i++)
    {
        if (fgets(line, sizeof(line), pData) == 0 ||
            (q->choices[i] = dupString(choices)) == 0)
            return EOF;
    }
    return 0;
}

int parseQuestion(FILE *pData, int qnum, QUESTION *q)
{
    printf("******** Question: %d ********\n", num);
    *q = (QUESTION){ NULL, NULL, 0, NULL };

    if (read_info(pData, &q->question) == EOF ||
        read_number(pData, &q->n_choices) == EOF ||
        (q->choices = calloc(q->n_choices, sizeof(char *))) == 0 ||
        read_choices(pData, q) == EOF ||
        read_info(pData, &q->correct_answer) == EOF)
    {
        free_choices(q);
        return EOF;
    }

    return 0;
}

You could have read_choices() call read_info() iteratively instead of implementing read_info() inside read_choices(). There's usually room for improvement in any code.