-2

Im working on function which reads text file. Function should find out how many records the text file have and malloc an array of structures. It must be array of structures because it is specified in my assignment. LineCounter holds value of how big the array should be. It has value 17 which is correct but when I malloc the array it only have size of 4 structs. I am programming in C and I am also including <stdlib.h> but the compiler is still forcing me to cast malloc.

// SEPARATE HEADER FILE
#pragma once
#define FILECITATEL "citatel.txt"   // textovy subor obsahujuci zoznam citatelov
typedef struct Citatel {
    int id;               // id cislo citatela
    char meno[20];        // meno citatela
    char priezvisko[30];  // priezvisko citatela        
};

Citatel *getReaders();
void printReaders(Citatel *listOfReaders);
// END OF HEADER FILE

Citatel *getReaders() {

    FILE *fileRead;

    fileRead = fopen(FILECITATEL, "r");
    if (fileRead == NULL) {
        printf("File cannot be opened!...");
        return NULL;
    }

    char readCharacter;
    int lineCounter = 0;

    while ((readCharacter = fgetc(fileRead)) != EOF) {
        if (readCharacter == '\n') lineCounter++;
    }

    if (readCharacter == EOF) lineCounter++;

    rewind(fileRead);

    Citatel *list = (Citatel *)malloc(lineCounter * sizeof(Citatel));

    for (int i = 0; i < lineCounter; i++) {
        fscanf(fileRead, "%d %s %s", &list[i].id, &list[i].meno, &list[i].priezvisko);
    }

    printf("%d\n", sizeof list);

    fclose(fileRead);

    return list;
}
chqrlie
  • 131,814
  • 10
  • 121
  • 189
  • 4
    Interestingly, you are asking about an issue involving malloc and an array of structs... Yet the code that you have pasted in does not have the definition of the structs. Also, pick a language. C or C++. If you are using C, do NOT cast the return from Malloc. – David Hoelzer Feb 07 '16 at 18:50
  • 3
    If you're using C++, use new, not malloc. – Donnie Feb 07 '16 at 18:50
  • Sorry my mistake Im using c and I need to cast it because otherwise visual studio is giving me error that malloc is undefined – Matúš Kačmár Feb 07 '16 at 18:54
  • There's a slight logical problem with your function: It might actually allocate (and read) one ŕecord to little. The problem is if the last line in the input file doesn't end with a newline you won't count it. – Some programmer dude Feb 07 '16 at 18:57
  • 3
    `char readCharacter` is wrong and results in trouble! See the manpage for `fgetc` which type it returns. Hint: if `char` is unsigned, `readCharacter == EOF` always fails. Enable compiler-warnings and pay heed to them. – too honest for this site Feb 07 '16 at 19:00
  • 1
    *"visual studio is giving me error that malloc is undefined"*. You must `#include ` – Weather Vane Feb 07 '16 at 19:05
  • Im including stdlib.h but it throws error cannot convert from void* to Citatel* – Matúš Kačmár Feb 07 '16 at 19:08
  • 1
    Well put that in your question please, as well as the code that faults and the definition of `Citatel`. You not *not* need to cast the return value from `malloc` in C except to force the compiler to accept an error. – Weather Vane Feb 07 '16 at 19:10
  • I cannot reproduce your problem with `malloc` although the `typedef` was not recognised until I rearranged it to be `typedef struct { ... } Citatel;` – Weather Vane Feb 07 '16 at 19:22
  • 2
    C is not C++ is not C! If `malloc` without casting fails, you use the wrong compiler. Never compile C code with a C++ or Java or C# compiler! – too honest for this site Feb 07 '16 at 19:29
  • You need to post a [Minimal, Complete, and Verifiable example](http://stackoverflow.com/help/mcve) so that your faults can be reproduced. For example you *claim* to have included `stdlib.h` and it resulted in several unnecessary comments. But we still don't know that it was included *and accessible* – Weather Vane Feb 07 '16 at 19:34
  • Ive included it right into that source file – Matúš Kačmár Feb 07 '16 at 19:46
  • Well ***I*** can't see it! – Weather Vane Feb 07 '16 at 19:55
  • Modify you post to have separate code blocks for the include file and the source file. `#pragma once` is non portable, The prototype and definition of `getReaders` should be `Citatel *getReaders(void);` – chqrlie Feb 07 '16 at 21:24

3 Answers3

2

You are understanding wrong:

sizeof list

return the size of the pointer not the size of the memory that you allocate.

2

The variable list is a pointer, when you get the size of list you get the size of the pointer and not what it points to. You need to keep track of the memory you allocate yourself.


And the usual link: Don't cast the return of malloc in C (or any other function returning void *).

And if you're programming in C++ then you should be using std::vector instead (or worst case use new[]) and the standard library streams.

Community
  • 1
  • 1
Some programmer dude
  • 400,186
  • 35
  • 402
  • 621
  • compiler is giving me the error if Im not casting before malloc because of stdlib.h which Im using otherwise If I dont use casting it throws error that malloc is undefined – Matúš Kačmár Feb 07 '16 at 18:58
  • 1
    @MatúšKačmár You *do* include ``? Follow the link, it will tell you what's so dangerous about casting the result without including the header file. If you include the header you don't need the cast. – Some programmer dude Feb 07 '16 at 19:00
  • Im including the header file but it behaves the same way nothing changed still throwing error cannot convert from Void* to Citatel* – Matúš Kačmár Feb 07 '16 at 19:11
  • 1
    @MatúšKačmár Then you are not actually programming in C, you're programming in C++, or at least use a C++ compiler! Change the source file extension from e.g. `.cpp` to plain `.c` and most compiler front-ends will compile it as a C source file. – Some programmer dude Feb 07 '16 at 19:14
  • Im using visual studio because we are forced to use it. It doesnt support plain c programs everything Im doing in c must have cpp extension. Im beginner in programming. Im casting malloc because that is only way I know how to manully allocate memmory. That is the way how they taught me to do it. I just want simple answer Is the problem caused because Im casting malloc or not ? If not how should I allocate array of structures – Matúš Kačmár Feb 07 '16 at 19:26
  • @MatúšKačmár The MSVC++ does indeed support C programs and C source files. Just rename the files and try again. The 2015 edition even have fixed much of its problems with more modern C variants (it's added just about all of C99). – Some programmer dude Feb 07 '16 at 19:27
  • 1
    Very important: Also change your definition of `readCharacter` to `int readCharacter;`. – chqrlie Feb 07 '16 at 21:21
2

Your mistake is in the line where you take sizeof list, because that prints the size of list pointer, which is 4 bytes. You want to print lineCounter instead.

Additionally, you are suggested to avoid casting the result of malloc. See here: Do I cast the result of malloc?

Community
  • 1
  • 1
kfx
  • 8,136
  • 3
  • 28
  • 52