Cannot append elements in struct array properly

Question

This is my first post here. I am making a program in C that handles Joker results (it's a game like Powerball). Below I include only the code that matters for my question. First, you input 1 to the program so that it reads the previous results file. I will include the file so that you can run it as well.Afterwards you input 3 so that you insert a result in the form of

id;day/month/year;num1;num2;num3;num4;num5;joker

After that you input 99 and you see the full result array. The problem is that the first 2 results that are appended to the array(resArr) are displayed properly, but all the following appends are stored with pseudorandom numbers. Any clue why my code works only for 2 repetitions? The file: link

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

 typedef struct results
 {
     int id,date[3],num[5],joker;
 }Results;

 Results *read()
 {
     FILE *fp=fopen("joker.csv","r");
     Results *temp=(Results *)malloc(sizeof(Results));
     Results *result=(Results *)malloc(sizeof(Results));
     int i=0,size=1;
     while(!feof(fp))
     {
         char *s=(char *)malloc(50*sizeof(char));
         fgets(s,50,fp);
 sscanf(s,"%d;%d/%d/%d;%d;%d;%d;%d;%d;%d",&result[i].id,&result[i].date[0],&result[i].date[1],&result[i].date[2],&result[i].num[0],&result[i].num[1],&result[i].num[2],&result[i].num[3],&result[i].num[4],&result[i].joker);
        temp=(Results *)realloc(result,(++size)*sizeof(Results));
        if (temp) result=temp;
        else
        {
            result=NULL;
            break;
        }
        i++;
    }
    fclose(fp);
    return result;
}

int findLength()
{
    FILE *fp=fopen("joker.csv","r");
    int len,i=0;
    while(!feof(fp))
    {
        char *s=(char *)malloc(50*sizeof(char));
        fgets(s,50,fp);
        i++;
    }
    fclose(fp);
    len=i-1;
    return len;
}

void eisagogi(Results *resArr,int *len)
{
    Results result;
    printf("id;dd/mm/yyyy;num1;num2;num3;num4;num5;joker\n");
scanf("%d;%d/%d/%d;%d;%d;%d;%d;%d;%d",&result.id,&result.date[0],&result.date[1],&result.date[2],&result.num[0],&result.num[1],&result.num[2],&result.num[3],&result.num[4],&result.joker);
    resArr=(Results *)realloc(resArr,(*len+1)*sizeof(Results));
    resArr[*len]=result;
    *len=*len+1;
}

void showResults(Results *resArr,int len)
{
    int i;
    for (i=0;i<len;i++)
    {        printf("%d;%d/%d/%d;%d;%d;%d;%d;%d;%d\n",resArr[i].id,resArr[i].date[0],resArr[i].date[1],resArr[i].date[2],resArr[i].num[0],resArr[i].num[1],resArr[i].num[2],resArr[i].num[3],resArr[i].num[4],resArr[i].joker);
    }
}

int menuChoose()
{
    int choice;
    printf("Load results 1\n");
    printf("Append result 3\n");
    printf("Result array 99\n");
    printf("Exit 0\n");
    scanf("%d",&choice);
    return choice;
}

int main()
{
    Results *resArr=(Results *)malloc(sizeof(Results));
    int choice,len;
    while(1)
    {
        choice=menuChoose();
        switch(choice)
        {
        case 1:
            resArr=read();
            len=findLength();
            break;
        case 3:
            eisagogi(resArr,&len);
            break;
        case 99:
            showResults(resArr,len);
            break;
        case 0:
            exit(0);
            break;
        }
    }
    return 0;
}

Can you show an example run of your program and explain how the output differs from what you want? — Code-Apprentice, Jan 02 '17 at 14:05
p.s. Be sure to `free()` any pointers that you `malloc()` or `realloc()`. — Code-Apprentice, Jan 02 '17 at 14:06
Fix your [use of feof()](http://stackoverflow.com/questions/5431941/why-is-while-feof-file-always-wrong). — Alexey Frunze, Jan 02 '17 at 14:07

score 3 · Accepted Answer · answered Jan 02 '17 at 14:35

I changed your feof to fgets

char s[50];
while (fgets(s, sizeof(s), fp) != 0) {

and now it seems that you can add 3 results and display them.

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

typedef struct results {
    int id, date[3], num[5], joker;
} Results;

Results *read() {
    FILE *fp = fopen("joker.csv", "r");
    Results *temp = (Results *) malloc(sizeof(Results));
    Results *result = (Results *) malloc(sizeof(Results));
    int i = 0, size = 1;
    char s[50];
    while (fgets(s, sizeof(s), fp) != 0) {
        sscanf(s, "%d;%d/%d/%d;%d;%d;%d;%d;%d;%d", &result[i].id, &result[i].date[0], &result[i].date[1],
               &result[i].date[2], &result[i].num[0], &result[i].num[1], &result[i].num[2], &result[i].num[3],
               &result[i].num[4], &result[i].joker);
        temp = (Results *) realloc(result, (++size) * sizeof(Results));
        if (temp) result = temp;
        else {
            result = NULL;
            break;
        }
        i++;
    }
    fclose(fp);
    return result;
}

int findLength() {
    FILE *fp = fopen("joker.csv", "r");
    int len, i = 0;
    while (!feof(fp)) {
        char *s = (char *) malloc(50 * sizeof(char));
        fgets(s, 50, fp);
        i++;
    }
    fclose(fp);
    len = i - 1;
    return len;
}

void eisagogi(Results *resArr, int *len) {
    Results result;
    printf("id;dd/mm/yyyy;num1;num2;num3;num4;num5;joker\n");
    scanf("%d;%d/%d/%d;%d;%d;%d;%d;%d;%d", &result.id, &result.date[0], &result.date[1], &result.date[2],
          &result.num[0], &result.num[1], &result.num[2], &result.num[3], &result.num[4], &result.joker);
    resArr = (Results *) realloc(resArr, (*len + 1) * sizeof(Results));
    resArr[*len] = result;
    *len = *len + 1;
}

void showResults(Results *resArr, int len) {
    int i;
    for (i = 0; i < len; i++) {
        printf("%d;%d/%d/%d;%d;%d;%d;%d;%d;%d\n", resArr[i].id, resArr[i].date[0], resArr[i].date[1], resArr[i].date[2],
               resArr[i].num[0], resArr[i].num[1], resArr[i].num[2], resArr[i].num[3], resArr[i].num[4],
               resArr[i].joker);
    }
}

int menuChoose() {
    int choice;
    printf("Load results 1\n");
    printf("Append result 3\n");
    printf("Result array 99\n");
    printf("Exit 0\n");
    scanf("%d", &choice);
    return choice;
}

int main() {
    Results *resArr = (Results *) malloc(sizeof(Results));
    int choice, len;
    while (1) {
        choice = menuChoose();
        switch (choice) {
            case 1:
                resArr = read();
                len = findLength();
                break;
            case 3:
                eisagogi(resArr, &len);
                break;
            case 99:
                showResults(resArr, len);
                break;
            case 0:
                exit(0);
                break;
        }
    }
    return 0;
}

Test

Load results 1
Append result 3
Result array 99
Exit 0
3
id;dd/mm/yyyy;num1;num2;num3;num4;num5;joker
1768;18/12/2016;11;28;5;9;31;1
Load results 1
Append result 3
Result array 99
Exit 0
3
id;dd/mm/yyyy;num1;num2;num3;num4;num5;joker
1769;18/12/2016;11;28;5;9;31;2
Load results 1
Append result 3
Result array 99
Exit 0
3
id;dd/mm/yyyy;num1;num2;num3;num4;num5;joker
1770;18/12/2016;11;28;5;9;31;3
Load results 1
Append result 3
Result array 99
Exit 0
1
...
1768;18/12/2016;11;28;5;9;31;1
1769;18/12/2016;11;28;5;9;31;2
1770;18/12/2016;11;28;5;9;31;3
Load results 1
Append result 3
Result array 99
Exit 0

score 1 · Answer 2 · edited May 23 '17 at 11:53

Your I/O strategy is one part fragile and one part flat wrong.

In the first place, while(!feof(fp)) is always wrong. As the answers to the linked question explain in some detail, the feof() function reports on whether end-of-file has already been detected on the specified stream. It cannot report on whether the next attempt to read will encounter EOF, so you need to watch for that on each read. In this case, you would want to verify that fgets() does not return NULL, as @DacSaunders already recommended:

char s[50];
while (fgets(s, sizeof(s), fp) != NULL) {

...

(The integer literal 0 also serves as a null pointer constant, but I find it clearer to use NULL.) Note also that Dac changed your s to an automatic array instead of having it dynamically allocated. That's not only easier to manage, but it cleans up the memory leak that your original code exhibited from dynamically allocating 50 bytes for each line and never freeing them. That also allows you to use sizeof(s) to get the capacity of s -- that works for arrays, but not for pointers.

In the second place, you do not check your inputs.

You should verify that each fgets() in fact reads a whole line (as can be determined by looking for a newline in the data read), for the next read will pick up where the last left off, and that could be in the middle of a line.
You should verify that each sscanf() in fact matches every expected field (by checking its return value), for if it does not do so then some or even all of the fields you are trying to populate will be uninitialized. You might have such a failure if the input is malformed, or even if an input line is just too long.

Bonus tip: you have another memory leak, in that you allocate memory for temp in its declaration, but you never free it, nor even use it. Just because you declare a pointer does not mean you have to allocate memory.

score 0 · Answer 3 · answered Jan 02 '17 at 16:11

0

Thanks a lot! I repeated Dac's change in findLength as well and I also used temp() to free temp in read(). Was this bug happening because of memory leaks?

answered Jan 02 '17 at 16:11

Answers are for answers, not for saying thanks. You can say thanks in a comment to an answer. – Alexey Frunze Jan 03 '17 at 08:05

Cannot append elements in struct array properly

3 Answers3