2010-05-15 15 views
6

hier j'avais posté une question: How should I pass a pointer to a function and allocate memory for the passed pointer from inside the called function?C Programmation: malloc() pour un tableau 2D (en utilisant le pointeur à pointeur)

A partir des réponses que j'ai, j'ai pu comprendre quelle erreur je faisais.

Je suis confronté à un nouveau problème maintenant, quelqu'un peut-il m'aider?

Je veux allouer dynamiquement un tableau 2D, donc je passe un pointeur à pointeur de mon main() à une autre fonction appelée alloc_2D_pixels(...), où j'utilise malloc(...) et for(...) boucle pour allouer de la mémoire pour le tableau 2D.

Eh bien, après le retour de la fonction alloc_2D_pixels(...), le pointeur à pointeur NULL reste encore, si naturellement, lorsque je tente d'accéder ou essayer de free(...) le pointeur à pointeur, le programme se bloque.

Quelqu'un peut-il me suggérer quelles erreurs je fais ici?

Aidez-moi !!!

Vikram


SOURCE:

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); 
} 

Répondre

3

Une erreur est l'écriture du code qui ne compilera pas :). le code est corrigé ci-dessous avec mes commentaires dans
/* ce style * /:

/* 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); 
} 
+0

Salut Mark !!! :) Ouais, tu as raison, j'aurais dû poster un code de travail. Merci pour votre réponse détaillée, je l'apprécie. – HaggarTheHorrible

2

Dans votre fonction alloc_2D_pixels, vous avez besoin d'un autre niveau d'indirection lors de l'accès memory. Comme c'est le cas maintenant, vous modifiez uniquement le paramètre, pas le pointeur pointé par le paramètre. Par exemple,

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)); 

(essentiellement, partout où vous utilisez memory, vous devez utiliser (*memory), les parenthèses ne sont nécessaires que lorsque vous utilisez des indices pour faire en sorte que les opérateurs sont appliqués dans l'ordre correct)

+1

Devrait être sizeof (unsigned char *) non non signé char ** aussi. –

+0

IMHO, il devrait être vraiment '* memory = malloc (rows * sizeof ** mémoire)' et '(* mémoire) [i] = malloc (cols * sizeof * (* mémoire) [i])', soit toujours un extra '*' sous le 'sizeof'. Beaucoup plus résistant aux erreurs. – AnT

1

Il ressemble également à, Vous utilisez non initialisé rows et cols variables

1

utilisant des tableaux multidimensionnels de cette manière dans C est pour la performance "suboptimale".

En mots non clairs: Veuillez ne pas utiliser - et certainement pas initialiser - les tableaux multidimensionnels comme vous l'avez illustré. Plusieurs appels à malloc() vous créeront un lot d'emplacements de mémoire disjoints qui ne correspondent pas à la façon dont les graphiques réels (en tant que tampons uniques contigus) sont stockés n'importe où. En outre, si vous devez le faire des centaines ou des milliers de fois, malloc() peut être horriblement cher.

De plus, étant donné que vous utilisez très souvent malloc(), c'est aussi un cauchemar (et un bug pour vous mordre finalement) pour le nettoyage. Vous l'avez même mentionné dans les commentaires de votre code, et pourtant ... pourquoi?

Si vous devez absolument avoir cette ptr[rows][cols] chose, créez mieux comme ceci:

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; 
} 

cette façon, vous allouons un seul bloc de la mémoire et l'ensemble peut être libéré en une seule fois - fossé free_2D_pixels().

+0

Ah, aurait été surpris si personne d'autre n'avait répondu à cela par le passé. Crédit où le crédit est dû: http://stackoverflow.com/questions/3144132/malloc-in-c-but-use-multi-dimensional-array-syntax/3144577#3144577 voir là. –