C - struct problems - writing

Question

Hello, I'm making a program in C, and I'mm having some troubles with memory, I think.

So my problem is: I have 2 functions that return a struct. When I run only one function at a time I have no problem whatsoever. But when I run one after the other I always get an error when writting to the second struct.

Function struct item* ReadFileBIN(char *name) -- reads a binary file. struct tables* getMesasInfo(char* Filename) -- reads a text file.

My code is this:

#include "stdafx.h"
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <time.h>

int numberOfTables=0;
int numberOfItems=0;

//struct tables* mesas;
//struct item* Menu;

typedef struct item{
    char nome[100];
    int id;
    float preco;
};
typedef struct tables{
    int id;
    int capacity;
    bool inUse;
};
struct tables* getMesasInfo(char* Filename){
    struct tables* mesas;
    char *c;
    int counter,numberOflines=0,temp=0;
    char *filename=Filename;
    FILE * G;
    G = fopen(filename,"r");
    if (G==NULL){
        printf("Cannot open file.\n");
    }
    else{
     while (!feof(G)){
     fscanf(G, "%s", &c);
     numberOflines++;
        }
    fclose(G);
    }  
    /* Memory allocate for input array */
    mesas = (struct tables *)malloc(numberOflines* sizeof(struct tables*));
    counter=0;
    G=fopen(filename,"r");
    while (!feof(G)){
        mesas[counter].id=counter;
        fscanf(G, "%d", &mesas[counter].capacity);
        mesas[counter].inUse= false;
        counter++;
    }
fclose(G);
numberOfTables = counter;
return mesas;
}

struct item* ReadFileBIN(char *name)
{
        int total=0;
        int counter;
        FILE *ptr_myfile;
        struct item my_record;
        struct item* Menu;
        ptr_myfile=fopen(name,"r");
        if (!ptr_myfile)
        {
            printf("Unable to open file!");
        }

        while (!feof(ptr_myfile)){
            fread(&my_record,sizeof(struct item),1,ptr_myfile);
            total=total+1;
        }
        numberOfItems=total-1;
        Menu = (struct item *)calloc(numberOfItems , sizeof(struct item));
        fseek(ptr_myfile, sizeof(struct item), SEEK_END);
        rewind(ptr_myfile);
        for ( counter=1; counter < total ; counter++)
        {
            fread(&my_record,sizeof(struct item),1,ptr_myfile);
            Menu[counter] = my_record;
            printf("Nome: %s\n",Menu[counter].nome);
            printf("ID: %d\n",Menu[counter].id);
            printf("Preco: %f\n",Menu[counter].preco);
        }
        fclose(ptr_myfile);
        return Menu;
}

int _tmain(int argc, _TCHAR* argv[])
{
    struct item* tt = ReadFileBIN("menu.dat");
    struct tables* t = getMesasInfo("Capacity.txt");
    getchar();
}**

the error that im getting is :

"Unhandled exception at 0x00411700 in test.exe: 0xC0000005: Access violation writing location 0x00000000."

in "Menu[counter] = my_record;"

Thanks in advance.

Answer 1

You seem to allocate a memory block of the wrong size in getMesasInfo(): sizeof(struct tables*) gives you the size of the pointer, not that of the struct it is pointing to:

mesas = (struct tables *)malloc(numberOflines* sizeof(struct tables*));

So you can easily overwrite unallocated memory. The proper allocation should be

mesas = (struct tables *)malloc(numberOflines* sizeof(struct tables));

or, similar to how you allocate the other array in ReadFileBIN():

mesas = (struct tables *)calloc(numberOflines, sizeof(struct tables));

Moreover, I don't know whether it's intentional or not, but in ReadFileBIN() you are allocating (1) and reading (2) one less record than the total number of records:

    numberOfItems=total-1;                                             // 1
    Menu = (struct item *)calloc(numberOfItems , sizeof(struct item)); // 1
    fseek(ptr_myfile, sizeof(struct item), SEEK_END);
    rewind(ptr_myfile);
    for ( counter=1; counter < total ; counter++)                      // 2
    ...

Since the loop counter is started from 1 (instead of 0 as is normal in C), you effectively execute the loop total-1 times. That is, you read into elements 1 to total-1 of the array. However, the real elements of the array are indexed from 0 to total-2 so the very first element in the array is left uninitialized, and in the end you commit a buffer overflow error.

Answer 2

To further illustrate Peter's point:

struct tables {
  int id;
  int capacity;
  int inUse; /* bool is not a C type */
};

int main()
{
  printf("sizeof: %d\n",sizeof(struct tables*));
  printf("sizeof: %d\n",sizeof(struct tables));

}

Will output:

sizeof: 4
sizeof: 12

Answer 3

You have a problem in these lines:

    numberOfItems=total-1;
    Menu = (struct item *)calloc(numberOfItems , sizeof(struct item));
    fseek(ptr_myfile, sizeof(struct item), SEEK_END);
    rewind(ptr_myfile);
    for ( counter=1; counter < total ; counter++)

First, you're setting numberOfItems to one less than the total. This is incorrect. You don't even need numberOfItems; since total has the number of lines in the file, the next line should really be Menu = (struct item*) calloc(total, sizeof(struct item));

Second, you're trying to use Menu as a one-based array in the for loop. C arrays are zero-based. You should have the for loop use for (counter = 0; counter < total; counter++).

Finally, as Peter pointed out, in the first function you're allocating the wrong size of object. You need to malloc numberOfLines*(sizeof(struct tables) (not sizeof(struct tables*).