What's the proper way to store a text file line by line in an array in C?

Question

I have a text file with variable length lines which I would like to store in a dynamic array using c.

The following is the dynamic array implementation:

struct array {
    char **elements;
    long size;
    long capacity;
};

void array_init(struct array *array, long capacity)
{
    array->capacity = capacity;
    array->elements = malloc(capacity * sizeof(char*));
}

void array_push_back(struct array *array, char *element)
{
    if (array->size == array->capacity) {
        array->capacity *= 2;
        array->elements = realloc(array->elements, array->capacity * sizeof(char*));
    }
    array->elements[array->size++] = element;
}

I read the file via the following:

struct array array;
FILE *file = fopen(filename, "r");
const unsigned MAX_LENGTH = 256;
char buffer[MAX_LENGTH];

array_init(&array, 1000);

while (fgets(buffer, MAX_LENGTH, file))
{
     array_push_back(&array, buffer);
}

for (int i = 0; i < array.size; i++)
{
     printf(array.elements[i]);
}

printf prints the last element only as many times as large the array is. I guess it is because I assign buffer's address to array.elements[i] and only change the content of buffer.

Can someone please help with correctly reading from a file and storing it?

where is your format specifier for printf? Maybe read the docs — OldProgrammer, May 31 '23 at 16:14
Doesn't `array.elements` just contain a whole bunch of pointers to the same buffer? — Chris, May 31 '23 at 16:18
`array->elements[array->size++] = element;` - it's the same pointer every time. — Ted Lyngmo, May 31 '23 at 16:19
Yeah this is the odd part `array->elements[array->size++] = element;`. You should copy contents of `buffer` (i.e., `element`) to `array->elements[array->size++]`. For instance, use `memcpy`. — Jardel Lucca, May 31 '23 at 16:19
Your method will store a line as multiple consecutive array elements if a line's length exceeds MAX_LENGTH. — Jeff Holt, May 31 '23 at 16:20
The function `array_init` should set `array->size` to `0`. Otherwise, it has an indeterminate variable (which may happen to be `0`, but may also be something else). — Andreas Wenzel, May 31 '23 at 16:26
Side note: Doubling the size of the array may not be the ideal growth rate. A better factor than `2.0` may be `1.5`. See this question for further information: [What is the ideal growth rate for a dynamically allocated array?](https://stackoverflow.com/q/1100311/12149471) — Andreas Wenzel, May 31 '23 at 16:39

score 4 · Accepted Answer · answered May 31 '23 at 16:19

4

array->elements[array->size++] = element;

You are storing a pointer to buffer from main over and over again to each element. You have to copy the content of the string.

array->elements[array->size++] = strdup(element);

or:

const size_t n = strlen(element);
array->elements[array->size] = malloc(n + 1);
memcpy(array->elements[array->size], element, n + 1);
array->size++;

answered May 31 '23 at 16:19

KamilCuk

120,984
8
59
111

`strdup` was a perfect fit in here since it both allocates memory and copies the string. – Jardel Lucca May 31 '23 at 16:27
1

It's very important to follow @Andreas Wenzel correction in the comments section too. – Jardel Lucca May 31 '23 at 16:30

Vlad from Moscow · Answer 2 · 2023-05-31T16:25:43.310

In the while loop in main

char buffer[MAX_LENGTH];

array_init(&array, 1000);

while (fgets(buffer, MAX_LENGTH, file))
{
     array_push_back(&array, buffer);
}

you are passing the same array buffer to the function array_push_back. So the function gets a pointer to the first element of array. And within the function all dynamically allocated pointers are set with this address

array->elements[array->size++] = element;

You need to allocate dynamically a character array for each pointer and copy the string stored in the array buffer into the dynamically allocated array.

Pay attention to that this function

void array_init(struct array *array, long capacity)
{
    array->capacity = capacity;
    array->elements = malloc(capacity * sizeof(char*));
}

does not initialize data member size to zero of the passed structure. It stays uninitialized.

And using the same pointer array->elements in the left and the right sides of the assignment in this statement

array->elements = realloc(array->elements, array->capacity * sizeof(char*));

is unsafe. The function can return a null pointer and the early allocated memory will be lost. You need to write something like

char **tmp = realloc(array->elements, 2 * array->capacity * sizeof(char*));

if ( tmp != NULL )
{
    array->elements = tmp;
    array->capacity  *= 2;
}

What's the proper way to store a text file line by line in an array in C?

2 Answers2