1

C Program crashes after last scanf/last loop, (trivia game).

struct myQuiz*quizInput(int *nrQ)
{
    int i, nrofrecords = 0;
    struct myQuiz *Quiz = (struct myQuiz*)malloc(sizeof (struct myQuiz)**nrQ);

    printf("How many Questions?\n");
    scanf_s("%d", nrQ);

    for (i = 0; i < *nrQ; i++) //for-loop för att skriva in påståenden
    {
        printf("Trivia input: ");
        fflush(stdin);
        fgets(Quiz[i].qQuest, 100, stdin);
        nrofrecords = nrofrecords + 1;
        fflush(stdin);

        printf("Enter answer '1'(yes) or '0' (no): ");
        scanf_s("%d", &Quiz[i].qAns);
        fflush(stdin);

        printf("Enter the difficulty (1-5)?: ");
        scanf_s("%d", &Quiz[i].qDiff);


    }
    return Quiz;
}
Rizier123
  • 58,877
  • 16
  • 101
  • 156
mrew
  • 21
  • 2
  • 1
    remove `fflush(stdin)` it's undefined behavior, what would it do anyway? You need to post more code, like the `struct` definition. – Iharob Al Asimi Jan 05 '15 at 23:30
  • 3
    Perform `struct myQuiz *Quiz = (struct myQuiz*)malloc(sizeof (struct myQuiz)**nrQ);` after `scanf_s("%d", nrQ);` – chux - Reinstate Monica Jan 05 '15 at 23:34
  • this line: 'struct myQuiz *Quiz = (struct myQuiz*)malloc(sizeof (struct myQuiz)**nrQ);' 1) allows for a problem related to extracting the max token size. Suggest: 'struct myQuiz *Quiz = malloc(sizeof (struct myQuiz)*(*nrQ));' Notice the parens to separate the multiply token from the dereference token 2) the returned value from malloc(and family) should not be cast 3) the returned value should always be checked to assure the memory allocation was successful. – user3629249 Jan 05 '15 at 23:45
  • the function: fgets() will consume/skipover leading white space, so there is no need to specifically consume those characters. (and as noted above, fflush(stdin); is undefined behaviour – user3629249 Jan 05 '15 at 23:52
  • regarding this line: 'printf("Enter answer '1'(yes) or '0' (no): ");' Answer yes/no to what? – user3629249 Jan 05 '15 at 23:54
  • this line: 'fgets(Quiz[i].qQuest, 100, stdin);' would be far better written as: fgets(Quiz[i].qQuest, sizeof(struct myQuiz.qQuest), stdin); I.E. do not use magic numbers when it can be avoided as they become a maintenance/debug nightmare. – user3629249 Jan 05 '15 at 23:58
  • the printf() statements fail to convey just what the user is expected to input. 1) 'trivia input' give no indication that the content of the question is to be entered. 2) 'Enter answer '1'(yes) or '0' (no): "' gives no indication that the user is to enter the expected answer 3) '"Enter the difficulty (1-5)?: "' there is no criteria for the 'difficulty' input. suggest revising the user prompts (the printf() statements) to inform the user of what is actually needed/what the answer means/ selection criteria – user3629249 Jan 06 '15 at 00:04
  • `fgets()` does not skip over leading white space. – chux - Reinstate Monica Jan 06 '15 at 01:09
  • [Please don't cast the resutl of `malloc()`](http://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc/605858#605858). – Quentin Jan 06 '15 at 02:13

3 Answers3

2

nRQ is the input to the function and memory allocated based on this value

struct myQuiz *Quiz = (struct myQuiz*)malloc(sizeof (struct myQuiz)**nrQ);

Actual questions value is asked after the malloc, so if initial value passed to the function is less than questions, the memory corruption happens. you need to get the input first and then allocate the memory later.

printf("How many Questions?\n");
scanf_s("%d", nrQ);
radar
  • 13,270
  • 2
  • 25
  • 33
1
the code is using some trash from nrQ for the malloc 
before nrQ variable is set.

returned values from I/O statements 
must be checked to assure successful operation.

the user prompts fail to clearly indicate what the user is to input

suggest:

    printf("Please indicate how many Questions you will enter?\n");
    if(1 != scanf_s("%d", nrQ))
    { // then, scanf_s failed
        perror( "scanf_s for number questions failed" );
        return( NULL );
    }

    // implied else, scanf_s successful

    struct myQuiz *Quiz = NULL;
    if( NULL == (Quiz = malloc(sizeof (struct myQuiz)*(*nrQ)) )) )
    { // then, malloc failed
        perror( "malloc array of struct myQuiz failed" );
        return( NULL );
    }

    // implied else, malloc successful

    printf("\nNote: max question length: %d\n", sizeof(struct myQuiz.qQuest) );
user3629249
  • 16,402
  • 1
  • 16
  • 17
0

You must initialize *nrQ first

struct myQuiz*quizInput(int *nrQ)
{
    int i, nrofrecords = 0;
    struct myQuiz *Quiz; // = malloc(sizeof (struct myQuiz)**nrQ); you need to initialize *nrQ first

    printf("How many Questions?\n");
    scanf_s("%d", nrQ);

    Quiz = malloc(sizeof (struct myQuiz)**nrQ);
    for (i = 0; i < *nrQ; i++) //for-loop för att skriva in påståenden
    {
        printf("Trivia input: ");

        fgets(Quiz[i].qQuest, 100, stdin);
        nrofrecords = nrofrecords + 1;

        printf("Enter answer '1'(yes) or '0' (no): ");
        scanf_s("%d", &Quiz[i].qAns);

        printf("Enter the difficulty (1-5)?: ");
        scanf_s("%d", &Quiz[i].qDiff);
    }
    return Quiz;
}    

Also

  1. Don't cast malloc
  2. Don't fflush(stdin)
Iharob Al Asimi
  • 52,653
  • 6
  • 59
  • 97