Quel est le problème dans cette fonction?

Question

Je suppose qu'il y a un problème avec la relation de malloc et goto. Ou, je suppose qu'il y a un gaspillage de mémoire ou une corruption de la mémoire qui se passe ici. J'espère que quelqu'un peut me signaler l'erreur exacte. Quand je compile son ne me donne pas d'erreur, mais, mon aîné insiste sur le fait que j'ai une erreur.

#define FINISH() goto fini; 

BOOL Do() 
{ 

    BOOL stat; 
    UINT32 ptr; 
    int err; 

    ptr = (UINT32)malloc(1000); 


    free((void*)ptr); 

fini: 
    return stat; 
} 

Answer 1

Voici les problèmes que je repéré dans le code

• Lorsque err != ERROR_SUCCESS cette fonction fuite de mémoire. Il sautera par-dessus l'appel free.
• Vous stockez le retour de malloc dans un emplacement 32 bits. Ce n'est pas une solution portable. Sur les plates-formes 64 bits, cela causera des ravages dans votre programme car vous tronquerez l'adresse. Si vous devez utiliser un type sans pointeur ici, utilisez plutôt size_t (bien que je recommanderais un pointeur sur un type intégral)
• Le stat local n'est pas affecté ici. Vous renvoyez des déchets si err != ERROR_SUCCESS. Il doit toujours recevoir une valeur. Le moyen le plus simple est de fournir une valeur par défaut.
• Vous ne cochez pas la valeur de retour de malloc et passer potentiellement un pointeur NULL caché dans Fun2

est ici la fonction avec les modifications que je suggérais

BOOL Do() 
{ 

    BOOL stat = FALSE; 
    size_t ptr = 0; 
    int err; 

    ptr = (UINT32)malloc(1000); 
    err = Fun1(); 

    if (err != ERROR_SUCCESS || ptr == 0) 
     FINISH(); 
    else 
     stat = Fun2(ptr); 

fini: 
    free((void*)ptr); 
    return stat; 
} 

Answer 2

malloc renvoie un pointeur. Vous lancez un pointeur sur un entier, mais les pointeurs et les entiers n'ont pas besoin d'avoir la même représentation. Par exemple, la taille du pointeur pourrait être de 64 bits et ne rentrerait pas dans votre entier.

L'objet stat peut également être utilisé non initialisé dans votre fonction. Sans explicitement initialisé, l'objet stat a une valeur indéterminée après sa déclaration.

Answer 3

Nous n'avons aucune idée de ce que cela est supposé faire, mais si Fun1() ne renvoie pas ERROR_SUCCESS, alors ptr n'est jamais libéré. Vraisemblablement, c'est l'erreur dont parle votre patron.

Answer 4

Vous transformez un pointeur sur un uint32_t et de retour. Ceci efface la moitié supérieure de la valeur de votre pointeur.

Answer 5

Quoi que vous fassiez, vous ne compilez pas ce code. Il a une erreur de syntaxe.

if(foo) 
    bar;; 
else 
    baz 

Vérifiez votre système de construction.

Answer 6

Mon commentaire général, et une règle générale pour travailler en C ... Si vous devez faire des jets de pointeurs, demandez-vous: est-ce vraiment nécessaire? Les moments où vous devrez honnêtement faire des jets de pointeurs sont extrêmement rares. Ce qui est beaucoup plus courant, c'est quand les gens utilisent des jets de pointeurs parce qu'ils ont des lacunes dans la compréhension, ne comprennent pas très clairement ce qu'ils essaient de faire ou devraient faire, et essaient de faire taire un avertissement du compilateur.

ptr = (UINT32)malloc(1000); 

Très mauvais! Si vous faites quelque chose avec ce "pointeur", vous serez très chanceux si cela fonctionne sur des plaforms 64 bits. Laissez les pointeurs comme types de pointeurs.Si vous devez absolument les stocker dans un nombre entier, utilisez uintptr_t qui est garanti être assez grand.

Je dirais que vous pourriez avoir essayé de le faire:

// Allocate 1,000 32-bit integers 
UINT32 *ptr = (UINT32*)malloc(1000 * sizeof(UINT32)); 

Cependant, même c'est la mauvaise forme de code C, un bizarre C et C++ hybride. Contrairement à C++, en C, vous pouvez simplement prendre un void * et apporter implicitement à tout type de pointeur:

// Allocate 1,000 32-bit integers 
UINT32 *ptr = malloc(1000 * sizeof(UINT32)); 

Enfin,

free((void*)ptr); 

casting à void* est un autre grand drapeau rouge, souvent un signe que la l'auteur ne sait pas ce qu'ils font. Une fois que vous changez ptr être un type de pointeur réelle, faites ceci:

free(ptr);