Programación C: malloc() para una matriz 2D (usando puntero a puntero)

Question

ayer que había publicado una pregunta: How should I pass a pointer to a function and allocate memory for the passed pointer from inside the called function?

partir de las respuestas que recibí, yo era capaz de entender lo error que hacía.

Estoy enfrentando un nuevo problema ahora, ¿alguien puede ayudarme con esto?

que desea asignar dinámicamente una matriz 2D, por lo que estoy pasando un puntero a puntero de mi main() a otra función llamada alloc_2D_pixels(...), donde uso malloc(...) y for(...) bucle para asignar memoria para la matriz 2D.

Bueno, después de regresar de la función alloc_2D_pixels(...), el puntero a puntero sigue siendo NULO, así que, naturalmente, cuando intento acceder o trato de free(...) Puntero-a-Puntero, el programa se bloquea.

¿Alguien puede sugerirme qué errores estoy haciendo aquí?

Ayuda !!!

Vikram

---

FUENTE:

main() 
{ 


unsigned char **ptr; 
unsigned int rows, cols; 

if(alloc_2D_pixels(&ptr, rows, cols)==ERROR)  // Satisfies this condition 
    printf("Memory for the 2D array not allocated"); // NO ERROR is returned 

if(ptr == NULL)     // ptr is NULL so no memory was allocated 
    printf("Yes its NULL!");   

// Because ptr is NULL, with any of these 3 statements below the program HANGS 
ptr[0][0] = 10;      
printf("Element: %d",ptr[0][0]); 

free_2D_alloc(&ptr); 

} 


signed char alloc_2D_pixels(unsigned char ***memory, unsigned int rows, unsigned int cols) 
{ 
     signed char status = NO_ERROR; 

     memory = malloc(rows * sizeof(unsigned char**)); 

     if(memory == NULL) 
     { 
      status = ERROR; 
      printf("ERROR: Memory allocation failed!"); 

     } 
     else 
     { 
      int i; 

      for(i = 0; i< cols; i++) 
      { 
       memory[i] = malloc(cols * sizeof(unsigned char)); 

       if(memory[i]==NULL) 
       { 
        status = ERROR; 
        printf("ERROR: Memory allocation failed!"); 
       } 
      } 

     } 

    // Inserted the statements below for debug purpose only 
     memory[0][0] = (unsigned char)10;  // I'm able to access the array from 
     printf("\nElement %d",memory[0][0]); // here with no problems 


     return status; 
} 


void free_2D_pixels(unsigned char ***ptr, unsigned int rows) 
{ 
    int i; 

    for(i = 0; i < rows; i++) 
    { 
      free(ptr[i]); 
    } 

    free(ptr); 
} 

Answer 1

Un error es publicar código que no se compila :). A continuación se corrige código con mis comentarios en
/* este estilo * /:

/* Next four lines get your code to compile */ 
#include <stdio.h> 
#include <stdlib.h> 
#define NO_ERROR 0 
#define ERROR 1 

/* prototypes for functions used by main but declared after main 
    (or move main to the end of the file */ 
signed char alloc_2D_pixels(unsigned char*** memory, unsigned int rows, unsigned int cols); 
void free_2D_pixels(unsigned char** ptr, unsigned int rows); 

/* main should return int */ 
int main() 
{ 
    unsigned char** ptr; 
    /* need to define rows and cols with an actual value */ 
    unsigned int rows = 5, cols = 5; 

    if(alloc_2D_pixels(&ptr, rows, cols) == ERROR) // Satisfies this condition 
     printf("Memory for the 2D array not allocated"); // ERROR is returned 

    if(ptr == NULL)     // ptr is NULL so no memory was allocated 
     printf("Yes its NULL!"); 
    else 
    { 
     /* Added else clause so below code only runs if allocation worked. */ 
     /* Added code to write to every element as a test. */ 
     unsigned int row,col; 
     for(row = 0; row < rows; row++) 
      for(col = 0; col < cols; col++) 
       ptr[0][0] = (unsigned char)(row + col); 

      /* no need for &ptr here, not returning anything so no need to pass 
       by reference */ 
      free_2D_pixels(ptr, rows); 
    } 

    return 0; 
} 

signed char alloc_2D_pixels(unsigned char*** memory, unsigned int rows, unsigned int cols) 
{ 
    signed char status = NO_ERROR; 

    /* In case we fail the returned memory ptr will be initialized */ 
    *memory = NULL; 

    /* defining a temp ptr, otherwise would have to use (*memory) everywhere 
     ptr is used (yuck) */ 
    unsigned char** ptr; 

    /* Each row should only contain an unsigned char*, not an unsigned 
     char**, because each row will be an array of unsigned char */ 
    ptr = malloc(rows * sizeof(unsigned char*)); 

    if(ptr == NULL) 
    { 
     status = ERROR; 
     printf("ERROR: Memory allocation failed!"); 
    } 
    else 
    { 
     /* rows/cols are unsigned, so this should be too */ 
     unsigned int i; 

     /* had an error here. alloced rows above so iterate through rows 
      not cols here */ 
     for(i = 0; i < rows; i++) 
     { 
      ptr[i] = malloc(cols * sizeof(unsigned char)); 

      if(ptr[i] == NULL) 
      { 
       status = ERROR; 
       printf("ERROR: Memory allocation failed!"); 
       /* still a problem here, if exiting with error, 
        should free any column mallocs that were 
        successful. */ 
      } 
     } 
    } 

    /* it worked so return ptr */ 
    *memory = ptr; 
    return status; 
} 


/* no need for *** here. Not modifying and returning ptr */ 
/* it also was a bug...would've needed (*ptr) everywhere below */ 
void free_2D_pixels(unsigned char** ptr, unsigned int rows) 
{ 
    /* should be unsigned like rows */ 
    unsigned int i; 

    for(i = 0; i < rows; i++) 
    { 
     free(ptr[i]); 
    } 

    free(ptr); 
} 

Answer 2

En su función alloc_2D_pixels, necesita otro nivel de indirección al acceder memory. Como está ahora, solo modifica el parámetro, no el puntero apuntado por el parámetro. Por ejemplo,

memory = malloc(rows * sizeof(unsigned char**)); 
// becomes 
*memory = malloc(rows * sizeof(unsigned char**)); 

// and later... 
memory[i] = malloc(cols * sizeof(unsigned char)); 
// becomes 
(*memory)[i] = malloc(cols * sizeof(unsigned char)); 

(básicamente, en cualquier lugar que esté utilizando memory, es necesario utilizar (*memory); los paréntesis sólo son necesarios cuando se utiliza subíndices para garantizar que los operadores se aplican en el orden correcto)

Answer 3

También parece que, está utilizando variables rows y cols no inicializadas

Answer 4

Utilización de matrices multidimensionales de esta forma en C es "óptimo" para el rendimiento.

Palabras no confusas: No utilice, y definitivamente no inicialice matrices multidimensionales en la forma que ha ilustrado. Las llamadas múltiples al malloc() le crearán un lote de ubicaciones de memoria disjuntas que no se correlaciona bien con la forma en que se almacenan los gráficos reales (como búferes contiguos y únicos) en cualquier lugar. Además, si tiene que hacerlo cientos o miles de veces, malloc() puede ser terriblemente costoso.

Además, debido al hecho de que está utilizando malloc() muy a menudo, también es una pesadilla (y error que puede morderlo) para limpiar. Incluso lo has mencionado en los comentarios en tu código, y aún ... ¿por qué?

Si es absolutamente necesario tener esta cosa ptr[rows][cols], lo crea mejor así:

signed char alloc_2D_pixels(unsigned char*** memory, 
          unsigned int rows, 
          unsigned int cols) 
{ 
    int colspan = cols * sizeof(char); 
    int rowspan = rows * sizeof(char*); 
    unsigned char **rowptrs = *memory = malloc(rowspan + rows * colspan)); 

    /* malloc failure handling left to the reader */ 

    unsigned char *payload = ((unsigned char *)rowptrs) + rowspan; 
    int i; 
    for (i = 0; i < rows; payload += colspan, i++) 
     rowptrs[i] = payload; 
} 

de esa manera se está asignando un único bloque de memoria y toda la cosa puede ser liberado de una sola vez - zanja free_2D_pixels().