Reallocating array of pointers.

[Ali3nSVK]

Hello, I'm having a slight problem with pointer arithmetics, specifically when trying to reallocate an array of pointers to wide character, previously allocated with malloc.

I have a 2D array declared inside a structure:

typedef struct list
{
    wchar_t **strings;
    int nodes;
} TList;

I've written a function that is supposed to open a file and read every line into a row in the array.

int getList(char *fname, TList *lst)
{
    wint_t c = 0; int csize = 8, asize = 8, i = 0, j = 0;


    FILE *fin = fopen(fname, "r");
    if (fin == NULL) return EFOPEN;


    lst->strings = malloc(asize * sizeof(wchar_t *));
    if (lst->strings == NULL) return ELOWMEM;


    while(c != WEOF)
    {
        lst->strings[i] = malloc(csize * sizeof(wchar_t));
        if (lst->strings[i] == NULL) return ELOWMEM;
        while((c = fgetwc(fin)) != L'\n' && c != WEOF)
        {
            lst->strings[i][j] = c;
            if (j == csize-1)
            {
                csize = csize<<1;
                lst->strings[i] = realloc(lst->strings[i], csize * sizeof(wchar_t));
                if (lst->strings[i] == NULL) return ELOWMEM;
            }
            j++;
        }
        lst->strings[i][j] = L'\0';
        if (i == asize)
        {
            asize = asize<<1;
            lst->strings = realloc(lst->strings, asize * sizeof(wchar_t *));
            if (lst->strings == NULL) return ELOWMEM;
        }
        j = 0; csize = 8;
        i++;
    }
    lst->nodes = i;
    fclose(fin);
    return EOK;

Variable csize represents the initial size of each row and asize represents the initial size of pointers to wchar. The function is working properly up to the point where the array of pointers to wchar_t needs to be reallocated. The initial values are small only for testing purposes. I will of course increase the initial size values later.

I have tried several methods including casting the pointer returned by malloc, or reallocating with sizeof(wchar_t **), but I've always received a segmentation fault, or NULL returned by realloc, which eventually led to Out of memory error.

Please excuse any mistakes, like not closing files before returning error code. The code is not yet finished.

Thank you very much for your help in advance.


EDIT.//

Also, I forgot to mention, that on 64bit Linux the code runs without errors, but when I run it through valgrind, it reports lots of unallocated space at exit even though I do clean up. On FreeBSD i386 and on Windows 7 64bit, the program immediately terminates with SIGSEGV.

[failure67]

If realloc fails, you are guaranteed a memory leak since you're not using a temporary variable. That's probably why you're getting an out of memory error.

[Ali3nSVK]

Yes, I will correct that part, thank you. But the main problem is it should not fail since I have lots of memory available.

[failure67]

I ran your getlist function and it seems to work correctly. Could you show us where you free up everything? There might be an error there.

[Ali3nSVK]

I'm not sure if thats the issue since I've tried commenting out the cleanup and it still receives SIGSEGV. Here is the function:

void cleanUp(TList *lst)
{
    for (int i = 0; i < lst->nodes; i++)
    {
        free(lst->strings[i]);
    }
    free(lst->strings);
}

As I've said it seems to run fine on 64b Linux, however it always crashes on win7 64b. Also the segfault
occurs only when there are more lines in the input file than the value of asize.

[failure67]

Here are two problems I found with your function. Use this as your input text:

\n
a

So line one is blank and line two contains the character 'a'. There is no line 3. Here's what your code would do:

First it would malloc space for the first line.

lst->strings[i] = malloc(csize * sizeof(wchar_t));

Second, it would see the newline character '\n'.

while((c = fgetwc(fin)) != L'\n' && c != WEOF)

Third, instead of placing '\n' into your array, you place '\0'.

lst->strings[i][j] = L'\0';

This is your first error. You just set a pointer to NULL without freeing its contents! Call free on strings[0] would do nothing on a Linux machine and you would have a memory leak.

Afterwards, your code does the 'a' character correctly. But let's see what it does after that.

j is incremented.

j++;

Then you see a newline character '\n'.

while((c = fgetwc(fin)) != L'\n' && c != WEOF)

Instead of putting the '\n' into your array, you put '\0'. This is totally fine here.

lst->strings[i][j] = L'\0';

Then you increment i

i++;

Then you go to your outer while loop.

while(c != WEOF)

Here c still equals '\n' and not WEOF. This is what begins your second error.

Now you malloc space that you don't need.

lst->strings[i] = malloc(csize * sizeof(wchar_t));

c becomes WEOF

while((c = fgetwc(fin)) != L'\n' && c != WEOF)

Now you set a pointer to null that you haven't freed yet! This is another memory leak.

lst->strings[i][j] = L'\0';

From there, your program runs fine. I don't know how these memory leaks cause a segfault on a Windows machine though.

[Ali3nSVK]

Hmm yes I understand, there are some errors I need to deal with, like the empty line, but for example:

lst->strings[i][j] = L'\0';

This line does not NULL the pointer. The whole inner loop puts the characters from a line into a separate row in the array
until it hits EOL and than L'\0' is put in, which simply indicates an end of string character. I need it here for functions like
fputws to work sice they output a string until they hit L'\0'. Also when I allocate space of ANY size for a row, I always keep
the number of nodes so I free it in the cleanup function no matter how big it is.

Take for example a file with simply a word "Hello" on each line, repeated for example 10 times. Does the code run for you fine ?
Again it does for me on Linux, but crashes on Win7 or FreeBSD.

[failure67]

This line does not NULL the pointer.

When j == 0, you are NULLing the pointer. j ends up being zero in two cases:

1) On blank lines
2) At the end of the file

2 is true because of the order in which you do everything.

Take for example a file with simply a word "Hello" on each line, repeated for example 10 times. Does the code run for you fine ?

No, your program is not running fine without errors, even on Linux. Yes, you are getting the correct output, but no, you are still setting a pointer to NULL and then freeing it.

I'll step you through it one more time. We're processing the 'o' in the tenth "Hello".

while((c = fgetwc(fin)) != L'\n' && c != WEOF) /*c is 'o' */

Everything in the inner loop runs fine and j is incremented. We now go to the next character.

while((c = fgetwc(fin)) != L'\n' && c != WEOF)/*c is now '\n'*/

The loop does not run and we skip to this line. This is totally fine because j is not zero.

lst->strings[i][j] = L'\0';/*No errors yet*/

Eventually we increment i.

i++;

Now we go back to the top of the outer loop. Here is where our error begins.

while(c != WEOF)/*Error: c is still '\n'*/

Then we malloc space.

lst->strings[i] = malloc(csize * sizeof(wchar_t));

Then we get to the inner loop. The loop does not execute.

while((c = fgetwc(fin)) != L'\n' && c != WEOF) /*c is now WEOF*/

Because the loop did not execute, j is zero.

lst->strings[i][j] = L'\0';/*Error: We are setting a pointer to NULL here! Memory Leak!*/

Eventually, you call free on this NULL pointer. On Linux machines, freeing a NULL pointer does nothing. Perhaps on other machines, it causes a segmentation fault.

[Salem]

> lst->strings[i][j] = L'\0';
> if (i == asize)
Look back at line 21, and think about why you had -1 in there.

Since you did lst->strings[asize][j] = L'\0';
You've been writing off the ends of your array.

Make lines 16 to 28 into a separate function that just returns a wchar_t* (or NULL) to the allocated line just read in.
It will simplify things no end.

[Ali3nSVK]

@Salem
That was exactly it! Thank you very much for your help, I'm not sure how I overlooked this so many times when debugging. Again, thank you.

@failure67
Thank you too for your time and patience with me on this, also thanks for lots of suggestions on improvements.

You guys saved me another sleepless night.