C String Allocation Wonkiness

Question

Having trouble understanding and getting to work String operations in the following code. Please help, me and my study colleagues are losing our minds over this. ty. This is a simple method to fill a multi dimensional array with custom strings - which for some reason we cannot figure out for the life of us does simply not work - spits out random junk from the memory instead. Also allocation amounts don't seem to be quite right.

#include <stdio.h>
#include <malloc.h>
#include <string.h>

char*** createKeyPad(int rows, int cols, int num_chars) {

    if(num_chars <= 0) return NULL;

    char needed = 'a';

    char** aptr = NULL;
    char*** rptr = NULL;

    aptr = (char**) malloc(rows * cols * sizeof(char*));
    if(aptr == NULL) {
        return NULL;
    }

    rptr = (char***) malloc(rows * sizeof(char**));
    if(rptr == NULL) {
        free(aptr);
        return NULL;
    }

    for(int row = 0; row < rows; row++) {
        rptr[row] = aptr + (row * cols);
    }

    for(int row = 0; row < rows; row++) {
        for(int col = 0; col < cols; col++) {
            char* string;
            for(int i = 0; i < num_chars; i++) {
                string[i] = needed;
            }
            string[num_chars] = '\0';
            rptr[row][col] = string;
            printf("%s", string);
        }
    }

    printf("%s", "hallo");
    return rptr;

}


int main() {
    printf("Hello, World!\n");
    char*** keypad = createKeyPad(5, 5, 3);

    for(int row = 0; row < 5; row++) {
        for(int col = 0; col < 5; col++) {
            printf("%s", keypad[row][col]);
        }
        printf("\n");
    }
   
}

Two things: There's no `` header in standard C, you need to include `` instead. And [in C you don't need (and really shouldn't) cast the result of `malloc`](https://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc). — Some programmer dude, Jan 28 '22 at 15:40
As for your problem, you have a pointer `string`, but *where does it **point***? — Some programmer dude, Jan 28 '22 at 15:42
@GammaNumeric "Having trouble understanding and getting to work String operations in the following code." And why are you trying to understand this bad code with undefined behavior? It is totally unclear. — Vlad from Moscow, Jan 28 '22 at 15:45
Obligatory [three-star programmer](https://wiki.c2.com/?ThreeStarProgrammer) joke. — HolyBlackCat, Jan 28 '22 at 16:03
All joking aside, when the first thing I see is a `***`, my immediate reaction is "Nope". That even though I have written code with triple pointers myself -- on a couple of rare occasions -- for specific and appropriate purposes. That many levels of indirection is rarely what you want. — John Bollinger, Jan 28 '22 at 16:29

0___________ · Answer 1 · 2022-01-28T16:02:03.970

You have plenty problems in this code.

string is a dangling pointer - ie it was not initialized and does not reference a valid char array
even if string was referencing a valid object you assign the same pointer to all (pseudo)array elements.
Do not use *** pointers.
use the correct type for sizes
Use positive checks and try to minimize function returns.
arrays are indexed from 0 in C and even if the string was referencing an array of num_chars elements, string[num_chars] = '\0'; is accessing an element outside the array bounds.
I would use array pointers and use only one allocation to allocate the space for the whole 3D array.
Use objects instead of types in sizeofs

int createKeyPad(size_t rows, size_t cols, size_t numChars, char (**pad)[cols][numChars]) 
{
    int result = 0;

    if(numChars > 1)
    {
        *pad = malloc(rows * sizeof(**pad));
        if(*pad)
        {
            result = 1;
            for(size_t row = 0; row < rows; row++) 
            {
                for(size_t col = 0; col < cols; col++) 
                {
                    for(size_t i = 0; i < numChars - 1; i++) 
                    {
                        (*pad)[row][col][i] = row * cols + col + '0';
                    }
                    (*pad)[row][col][numChars - 1] = 0;
                }
            }

        }
    }
    return result;
}


int main(void) 
{
    printf("Hello, World!\n");
    char (*keypad)[5][3];

    if(createKeyPad(5, 5, 3, &keypad))
    {
        for(size_t row = 0; row < 5; row++) 
        {
            for(size_t col = 0; col < 5; col++) 
            {
                printf("%s ", keypad[row][col]);
            }
            printf("\n");
        }
    }
    free(keypad);
}

https://godbolt.org/z/6zY4zbGW3

score 0 · Answer 2 · answered Jan 28 '22 at 16:20

The main problem is that char* string; followed by string[i] = needed; is dereferencing an invalid pointer because string is not pointing to anything valid.

In the code's current style of allocating one block for each level and dividing the block up, the memory for all the strings could be allocated in one big block:

char* sptr = (char*) malloc(rows * cols * (num_chars + 1) * sizeof(char));

(Note: The (char*) typecast is not required. Also the * sizeof(char) is not required since sizeof(char) is 1 by definition, but I put it in there in case the code is changed to use something other than char at a later date.)

Then the string variable in the nested loop can be initialized as follows:

char* string = sptr + (row * cols + col) * (num_chars + 1);

C String Allocation Wonkiness

2 Answers2