1

I've got a problem when dealing with one- and two-dimensional dynamically allocated arrays in C. I have found similar questions, however none of them helped me so I'm asking a new one.

Here is my problem. With the function below I want to add a new element into a two dimensional char array. Size is not the problem, I increment somewhere outside.

void addElem(char ***tabs, int size, char *new) {
    *tabs[size] = (char*)malloc(sizeof(char)*strlen(new));
    *tabs[size] = new;
    *tabs = (char **)realloc(*tabs, size * sizeof(char *));
}

In the code the function is called this way:

int i;
char *tmp;
char **array = (char **)malloc(sizeof(char *));
//some stuff, also initializing i and tmp
addElem(&array, i, tmp);

The problem is that the function even if allocates the data, it does not save any data there (for example, strcmp gives SIGSEGV using **array). When debugging similar functions (managing one-dim int arrays), I found out that they in fact work properly inside these functions, the data is just not saved after returning.

I tried to show as small amount of code as possible, but if it helps here is the full code (with addElem function renamed to dodajDoTablicySlow). Thanks.

Community
  • 1
  • 1
rafal.sz
  • 68
  • 9

2 Answers2

3

Consider your addElem() function:

void addElem(char ***tabs, int size, char *new) {
    *tabs[size] = (char*)malloc(sizeof(char)*strlen(nowy));
    *tabs[size] = new;
    *tabs = (char **)realloc(*tabs, size * sizeof(char *));
}

This function doesn't write the new element where you think it does. The array subscripting operator ([]) has higher precedence than the indirection operator (*), so *tabs[size] means "the thing pointed to by element size of array tabs", not "element size of the array pointed to by tabs".

Also, if size is the current size of the dynamic array -- as in the number of char * members *tabs has space to accommodate -- then (*tabs)[size] is beyond its capacity. Even if it requested sufficient space to include that position, which it doesn't, the realloc() call would not in any way be guaranteed to expand the memory allocated for *tabs to include that space. It is entirely free to put the new allocation somewhere else instead. It may sometimes need to do so when the new size is larger than the old, but it is free to do so even when the new size is smaller. At some point, uses of this function are likely to lose data in the reallocation.

In addition, your function leaks memory. It malloc()s space almost sufficient for the string pointed to by global variable nowy, then overwrites the only pointer to that space with function argument new.

Furthermore, your function has no good way to signal failure.

This would be a correct, safer, non-leaky implementation:

/*
 * Ensures that the dynamic array of char * pointed to by '*tabs' has
 * sufficient capacity for exactly 'size' + 1 elements, and stores the
 * value of 'el' at the last position.  Updates *tabs with a pointer to
 * the (possibly different) location of the enlarged array.  Returns the
 * location of the array, or NULL if memory allocation fails (in which
 * case '*tabs' is unchanged, and still a valid pointer to allocated memory).
 */
char **addElem(char ***tabs, size_t size, char *el) {
    char **newtabs = realloc(*tabs, (size + 1) * sizeof(char *));

    if (newtabs) {
        *tabs = newtabs;
        (*tabs)[size] = el;
    }

    return newtabs;
}
John Bollinger
  • 160,171
  • 8
  • 81
  • 157
  • Thanks for the explanation! Sorry, I forgot to say - the size is not the problem, I just increment it outside of that function (not the best way to do it, but I had no idea how to get rid of warnings when I was incrementing the size inside function). Also, there is my another fault - I did not change _nowy_ to _new_. I'll edit that quickly, sorry for that. – rafal.sz Apr 15 '15 at 21:12
  • On the contrary, `size` *is* one of the problems. Your original code definitely at least writes outside the bounds of the `realloc()`ed array, and probably outside the bounds of the starting allocation. Incrementing `size` outside the function has nothing to do with that. – John Bollinger Apr 15 '15 at 21:20
  • Moreover, just changing `nowy` to `new` still leaves you with a memory leak. If you want to make a copy of the string `new` points to and assign that to an array element, then consider using `strdup()`. In that case, remember that that memory will need to be freed later. – John Bollinger Apr 15 '15 at 21:22
  • Side note, but what a great example of how a question should be answered. – Alex Apr 15 '15 at 21:24
0

A very simple program about triple pointer:

#include <stdio.h>
void printElem(int ***tabs) {
    printf("%d\n",***tabs);

}
int main()
{
     printf("Hello, World!\n");
     int b = 7;
     int *d;
     int **t;
     d = &b;
     t = &d;
     printElem(&t);

     return 0;
}


output:
sh-4.3# main                                                                                                                                                                                                  
Hello, World!                                                                                                                                                                                                 
 7
ashish
  • 361
  • 3
  • 8