There's a number of issues.
Element * data[amountOfLines];
parsingFile(ignData, data);
data
is allocated but never initialized. It will contain garbage. So *data[i]
is dereferencing garbage. You either need to pre-initialize data
with Elements, or make parsingFile
responsible for allocating Elements and placing them on data
.
As the caller does not know how many lines will be in parsingFile
, it would also make sense to let parsingFile
allocate and return the Elements array and their elements.
fgets(buffer, maxLineLength, file);
char * token = strtok(buffer, ",");
strcpy((*data[i]).title, token);
The maxLineLength
is 150. buffer
and thus token
could be as large as 150 bytes, but title
can only hold 60.
Instead of hard coding the size of the title, make the title a character pointer and strdup
the token.
typedef struct {
char *title;
} Element;
...
// Use the size of the buffer so the buffer size and read size are
// never out of sync.
fgets(buffer, sizeof(buffer), file);
char * token = strtok(buffer, ",");
(*data[i]).title = strdup(token);
Here's one way to rewrite this code. Rather than using fixed size constants, or awkwardly passing around arrays and their size, it uses a struct to collect them together. All operations on that struct are in functions to make it easy to work with and debug.
#include<string.h>
#include<stdlib.h>
#include<stdio.h>
#include<assert.h>
typedef struct {
// Store a pointer rather than a fixed size string.
char *title;
} Element;
// A single struct to encapsulate the array and it size.
typedef struct {
// Store a list of pointers, not copies of the elements.
//
// This could be made generic by storing a `void **`, but that's advanced.
Element **data;
// Also store the size of the data.
size_t size;
} Elements;
// Have a function for creating structs, even if it's simple now.
// It might get complicated later. This is separate from initializing.
// For example, we don't allocate memory for title. That would be a
// waste.
Element *Element_new() {
return malloc(sizeof(Element));
}
Elements *Elements_new() {
return malloc(sizeof(Elements));
}
void Elements_init(Elements *elements) {
elements->data = NULL;
elements->size = 0;
}
Element *Elements_get(Elements *elements, size_t i) {
assert( i < elements->size);
return elements->data[i];
}
// Isolate the error-prone process of growing the array.
// This simplifies using Elements and offers an opportunity to
// optimize later.
void Elements_grow(Elements *elements, size_t by) {
size_t new_size = elements->size + by;
elements->data = realloc(elements->data, sizeof(Element*) * new_size);
elements->size += by;
}
// Same here, isolate a discrete process.
void Elements_append(Elements *elements, Element *element) {
Elements_grow(elements, 1);
elements->data[elements->size-1] = element;
}
Elements *parsingFile(FILE * file) {
// Use descriptive names.
Elements *elements = Elements_new();
Elements_init(elements);
// Allocate the buffer outside the loop to be reused.
// An optimizer will probably do this anyway, but it's
// easier to understand.
//
// Use BUFSIZ. I/O happens in blocks and BUFSIZ is often
// close to the block size.
char buffer[BUFSIZ];
// Read until you're done, not a fixed number of times.
//
// Use the size of the buffer so the buffer and read sizes will
// never get out of sync.
while( fgets(buffer, sizeof(buffer), file) != NULL ) {
char *token = strtok(buffer, ",");
// We don't need to initialize this Element because we're
// going to replace the title with our own pointer anyway.
Element *elem = Element_new();
// Copy the token from the buffer to the struct.
// This uses exact enough memory.
elem->title = strdup(token);
Elements_append(elements, elem);
}
return elements;
}
int main() {
FILE *ignData = fopen("t4_ign.csv", "r");
Elements *elements = parsingFile(ignData);
for( size_t i = 0; i < elements->size; i++ ) {
printf("%zu: %s\n", i, Elements_get(elements, i)->title);
}
}