Mallocing int* inside of int** gives unexpected integer values in the first and sometimes second allocation

Question

I am having this problem. Basically I want to parse an integer string to an int** where the sub int arrays are two long. The first index is the digit and the second is the power of ten associated to the digit.

For example 12345 would give [[1, 4], [2, 3], [3, 2], [4, 1], [5, 0]]

Here is the code:

#include <stdlib.h>

int    ft_strlen(char *str)
{
    int    count;

    count = 0;
    while (*str++)
        count++;
    return (count);
}

int **parray(char *str)
{
    int **ret;
    int len;
    int i;

    len = ft_strlen(str);
    ret = (int **)malloc(sizeof(int) * len);
    if (ret == NULL)
        return (NULL);
    i = 0;
    while (i < len)
    {
        ret[i] = (int *)malloc(sizeof(int) * 2);
        if (ret[i] == NULL)
            return (NULL);
        ret[i][0] = str[i] - '0';
        ret[i][1] = len - 1 - i;
        i++;
    }
    return (ret);
}

This then get tested in a main function as follows:

#include <stdio.h>

int main()
{
    char *str = "1255555555555555";
    int**ret = parray(str);
    int i = 0;
    while (i < ft_strlen(str))
    {
        printf("%d ", ret[i][0]);
        printf("%d ", ret[i][1]);
        printf("\n");
        i++;
    }
}

This for some reason prints out:

29791280 0 
29791408 0 
5 13 
5 12 
5 11 
5 10 
5 9 
5 8 
5 7 
5 6 
5 5 
5 4 
5 3 
5 2 
5 1 
5 0 

Note that the program works fine with strings less or equal to 4 digits. The first two non working values also always change when u run the program again. Im assuimng there is some overflow somewhere but I don't know where, nor how. Im fairly new to c so please excuse me if this is a silly mistake.

Answer 1

ret = (int **)malloc(sizeof(int) * len); is wrong. The type of ret is int **. It is a pointer to int *. You are allocating space for int *. So you want the size of int *.

Generally, it is preferable to calculate the size from the thing you are assigning to:

ret = malloc(len * sizeof *ret);`

*ret is a sample of what ret will point to, so sizeof *ret is the size of one of those things. In this case, it will be the size of an int *. This form automatically adjusts the allocation size to whatever type ret is. For example, if you change the declaration of ret later, while working on your program, the size here will automatically be correct without needing another edit.

Also, do not cast the return value of malloc in C. (C++ is different.) There is no need for it, and it can mask a failure to declare malloc (because an include of a header was accidentally omitted).

Answer 2

Coding a two-dimensional, dynamically allocated array of int is a non-trivial task. If you use a double pointer, there are significant memory allocation details. In particular, you need to check every allocation attempt, and, if any one fails, back out of the ones that didn't fail, without leaking memory. And, finally, to take a term from the C++ lexicon, you need to provide some equivalent of the Rule of Three functions used by a C++ RAII class.

A two-dimensional array of int with n_rows and n_cols should probably be allocated with a single call to malloc. For the program in the OP, you might have:

char *str = "1255555555555555";
const size_t n_rows = strlen(str);
const size_t n_cols = 2;
int (*data)[n_cols] = malloc(n_rows * sizeof (*data));

Variable data is a pointer to an array of 2 ints, i.e., it is a pointer to a row in the OP's two-dimensional array. sizeof(*data), therefore, is the size of one row. Multiplying by n_rows gives you the size of the entire array, which is the size allocated by malloc. So, malloc allocates the entire array, and variable data picks up a pointer to the first row.

See this Stack Overflow question for a fuller explanation of this C idiom. Also, see this question.

The program in the OP, however, does not use this idiom. It takes the double pointer route, and allocates an array of pointers to int. The elements in such an array are int*. Thus, the array is an array of pointers to rows, where each row is an array of int. The argument to the sizeof operator, therefore, must be int*.

int** data = malloc(n_rows * sizeof (int*));  // step 1.

Afterwards, you run a loop to allocate the columns of each row. Conceptually, each row is an array of int, with malloc returning a pointer to the first element of each row.

for (size_t r = 0; r < n_rows; ++r)
    data[r] = malloc(n_cols * sizeof int);  // step 2.

The program in the OP errs in step 1.

// from the OP:
ret = (int **)malloc(sizeof(int) * len);  // should be sizeof(int*)

Taking guidance from the answer by @Eric Postpischil, this can be coded as:

int** ret = malloc(len * sizeof *ret);

The program below, however, uses (the equivalent of):

int** ret = malloc(len * sizeof(int*));

Fixing memory leaks

The program in the OP is careful to check each call to malloc, and abort the allocation process if any one of them fails. After a failure, however, it does not call free to release the allocations that were successful. Potentially, therefore, it leaks memory. It should keep track of the row where a failure occurs, and call free on the preceding rows. It should also call free on the original array of pointers.

There is enough detail to warrant refactoring the code to create a separate header with functions to manage a two-dimensional array. This separates the business of array management from the application itself.

Header tbx.int_array2D.h, defined below, provides the following functions.

• Struct – struct int_array2D_t holds a data pointer, along with variables for n_rows and n_cols.
• Make – Function make_int_array2D handles allocations, and returns a struct int_array2D_t object.
• Free – Function free_int_array2D handles deallocations.
• Clone – Function clone_int_array2D returns a deep copy of a struct int_array2D_t object. It can be used in initialization expressions, but, in general, should not be used for assignments.
• Swap – Function swap_int_array2D swaps two int_array2D_t objects.
• Copy assign – Function copy_assign_int_array2D replaces an existing int_array2D_t object with a deep copy of another. It performs allocation and deallocation, as needed.
• Move assign – Function move_assign_int_array2D deallocates an existing int_array2D_t object, and replaces it with a shallow copy of another. After assignment, it zeros-out the source.
• Equals – Function equals_int_array2D performs a deep comparison of two int_array2D_t objects, returning 1 when they are equal, and 0, otherwise.

#pragma once
// tbx.int_array2D.h
#include <stddef.h>

struct int_array2D_t
{
    int** data;
    size_t n_rows;
    size_t n_cols;
};
void free_int_array2D(
    struct int_array2D_t* a);

struct int_array2D_t make_int_array2D(
    const size_t n_rows, 
    const size_t n_cols);

struct int_array2D_t clone_int_array2D(
    const struct int_array2D_t* a);

void swap_int_array2D(
    struct int_array2D_t* a,
    struct int_array2D_t* b);

void copy_assign_int_array2D(
    struct int_array2D_t* a,
    const struct int_array2D_t* b);

void move_assign_int_array2D(
    struct int_array2D_t* a,
    struct int_array2D_t* b);

int equals_int_array2D(
    const struct int_array2D_t* a, 
    const struct int_array2D_t* b);

// end file: tbx.int_array2D.h

A trivial application

With these functions, code for the application becomes almost trivial.

I am not sure why the OP wrote his own version of strlen, but I went with it, changing only the type of its return value.

// main.c
#include <stddef.h>
#include <stdio.h>
#include "tbx.int_array2D.h"

size_t ft_strlen(char* str)
{
    size_t count = 0;
    while (*str++)
        count++;
    return (count);
}

struct int_array2D_t parray(char* str)
{
    size_t len = ft_strlen(str);
    struct int_array2D_t ret = make_int_array2D(len, 2);
    for (size_t r = ret.n_rows; r--;) {
        ret.data[r][0] = str[r] - '0';
        ret.data[r][1] = (int)(len - 1 - r);
    }
    return ret;
}

int main()
{
    char* str = "1255555555555555";
    struct int_array2D_t ret = parray(str);
    for (size_t r = 0; r < ret.n_rows; ++r) {
        printf("%d %d \n", ret.data[r][0], ret.data[r][1]);
    }
    free_int_array2D(&ret);
}
// end file: main.c

Source code for tbx.int_array2D.c

Function free_int_array2D has been designed so that it can be used for the normal deallocation of an array, such as happens in function main, and also so that it can be called from function make_int_array2D, when an allocation fails.

Either way, it sets the data pointer to NULL, and both n_rows and n_cols to zero. Applications that use header tbx.int_array2D.h can check the data pointer of objects returned by functions make_int_array2D, clone_int_array2D, and copy_assign_int_array2D. If it is NULL, then the allocation failed.

// tbx.int_array2D.c
#include <stddef.h>
#include <stdlib.h>
#include "tbx.int_array2D.h"

//======================================================================
// free_int_array2D
//======================================================================
void free_int_array2D(struct int_array2D_t* a)
{
    for (size_t r = a->n_rows; r--;)
        free(a->data[r]);
    free(a->data);
    a->data = NULL;
    a->n_rows = 0;
    a->n_cols = 0;
}
//======================================================================
// make_int_array2D
//======================================================================
struct int_array2D_t make_int_array2D(
    const size_t n_rows, 
    const size_t n_cols)
{
    struct int_array2D_t a = {
        malloc(n_rows * sizeof(int*)),
        n_rows,
        n_cols
    };
    if (!n_rows || !n_cols)
    {
        // If size is zero, the behavior of malloc is implementation-
        // defined. For example, a null pointer may be returned. 
        // Alternatively, a non-null pointer may be returned; but such 
        // a pointer should not be dereferenced, and should be passed 
        // to free to avoid memory leaks. – CppReference
        // https://en.cppreference.com/w/c/memory/malloc

        free(a.data);
        a.data = NULL;
        a.n_rows = 0;
        a.n_cols = 0;
    }
    else if (a.data == NULL) {
        a.n_rows = 0;
        a.n_cols = 0;
    }
    else {
        for (size_t r = 0; r < n_rows; ++r) {
            a.data[r] = malloc(n_cols * sizeof(int));
            if (a.data[r] == NULL) {
                a.n_rows = r;
                free_int_array2D(&a);
                break;
            }
        }
    }
    return a;
}
//======================================================================
// clone_int_array2D
//======================================================================
struct int_array2D_t clone_int_array2D(const struct int_array2D_t* a)
{
    struct int_array2D_t clone = make_int_array2D(a->n_rows, a->n_cols);
    for (size_t r = clone.n_rows; r--;) {
        for (size_t c = clone.n_cols; c--;) {
            clone.data[r][c] = a->data[r][c];
        }
    }
    return clone;
}
//======================================================================
// swap_int_array2D
//======================================================================
void swap_int_array2D(
    struct int_array2D_t* a,
    struct int_array2D_t* b)
{
    struct int_array2D_t t = *a;
    *a = *b;
    *b = t;
}
//======================================================================
// copy_assign_int_array2D
//======================================================================
void copy_assign_int_array2D(
    struct int_array2D_t* a,
    const struct int_array2D_t* b)
{
    if (a->data != b->data) {
        if (a->n_rows != b->n_rows || a->n_cols != b->n_cols) {
            free_int_array2D(a);
            *a = make_int_array2D(b->n_rows, b->n_cols);
        }
        for (size_t r = a->n_rows; r--;) {
            for (size_t c = a->n_cols; c--;) {
                a->data[r][c] = b->data[r][c];
            }
        }
    }
}
//======================================================================
// move_assign_int_array2D
//======================================================================
void move_assign_int_array2D(
    struct int_array2D_t* a,
    struct int_array2D_t* b)
{
    if (a->data != b->data) {
        free_int_array2D(a);
        *a = *b;
        b->data = NULL;
        b->n_rows = 0;
        b->n_cols = 0;
    }
}
//======================================================================
// equals_int_array2D
//======================================================================
int equals_int_array2D(
    const struct int_array2D_t* a, 
    const struct int_array2D_t* b)
{
    if (a->n_rows != b->n_rows ||
        a->n_cols != b->n_cols) {
        return 0;
    }
    for (size_t r = a->n_rows; r--;) {
        for (size_t c = a->n_cols; c--;) {
            if (a->data[r][c] != b->data[r][c]) {
                return 0;
            }
        }
    }
    return 1;
}
// end file: tbx.int_array2D.c