Libérer de la mémoire, tout?

Question

Peut-être un mauvais sujet, mais étant donné le code suivant, dois-je aussi free(player->name)?

#include <stdio.h> 

struct Player 
{ 
     char *name; 
     int level;  
}; 

int main() 
{ 
    struct Player *player; 
    player->name = malloc(sizeof(player->name)*256); 
    player->name = "John"; 

    printf(player->name); 

    free(player); 

    printf("\n\n\n"); 
    system("PAUSE"); 
    return 0; 
} 

Answer 1

Oh, par où commencer? Tu as vraiment besoin d'un bon livre. Soupir. Commençons par le haut de main():

---

Ce

struct Player *player; 

définit un pointeur vers un struct Player, mais il n'initialise pas. Il a donc une valeur plus ou moins aléatoire, pointant quelque part dans la mémoire. Cette

player->name = malloc(sizeof(player->name)*256); 

écrit maintenant dans des parties de cet endroit au hasard l'adresse d'un morceau de mémoire obtenue par malloc(). L'écriture dans la mémoire via un pointeur non initialisé appelle Comportement indéfini. Après cela, tous les paris sont désactivés. Pas besoin de regarder plus bas dans votre programme. Vous êtes malchanceux d'avoir, par accident, écrit dans un morceau de mémoire qui appartient à votre processus, de sorte qu'il ne plante pas immédiatement, vous rendant conscient du problème.

Vous pouvez améliorer cela de deux façons. Soit coller au pointeur et le faire pointer vers une partie de la mémoire allouée pour un objet Player. Vous pouvez l'obtenir en appelant le malloc(sizeof(Player).
Ou tout simplement utiliser un local, objet automatique (basé pile alias):

struct Player player; 

Le compilateur va générer le code pour allouer de la mémoire sur la pile pour elle et dégagera automatiquement. C'est le plus facile, et devrait certainement être votre défaut.

---

Cependant, votre code a plus de problèmes que cela.

Ce

player->name = malloc(sizeof(player->name)*256); 

alloue la mémoire consécutive sur le tas pour stocker 256 pointeurs vers des caractères, et assigne l'adresse du premier pointeur (l'adresse d'un char*, donc un char**) à player->name (a char*). Franchement, je suis surpris que même compile, mais je suis plus habitué à l'application de type plus stricte de C++. Quoi qu'il en soit, ce que vous voulez sans doute à la place la place est d'allouer de la mémoire pour 256 caractères:

player->name = malloc(sizeof(char)*256); 

(Depuis sizeof(char) est, par définition, 1, vous verrez souvent comme malloc(256).)
Cependant, il y a plus à cette : Pourquoi 256? Que faire si je passe une chaîne de 1000 caractères? Non, il ne suffit pas d'allouer de l'espace pour une chaîne plus longue, car je pourrais lui passer une chaîne plus longtemps. Donc, soit 1) fixer la longueur maximale de la chaîne (juste déclarer Player::name être un tableau char de cette longueur, au lieu d'un char*) et de faire respecter cette limite dans votre code, ou 2) trouver la longueur nécessaire dynamiquement, à run-time, et allouer exactement la mémoire nécessaire (longueur de la chaîne plus 1, pour le '\0' char).

Mais ça devient pire. Cette

player->name = "John"; 

attribue alors l'adresse d'une chaîne littérale player->name, remplaçant l'adresse de la mémoire allouée par malloc() dans la seule variable que vous stockez, vous faire perdre la mémoire et de fuite.
Mais les chaînes ne sont pas des citoyens de première classe en C, donc vous ne pouvez pas les assigner. Ce sont des tableaux de caractères, par convention terminés par un '\0'. Pour les copier, vous devez les copier caractère par caractère, soit en boucle, soit de préférence en appelant le strcpy().

Pour ajouter l'insulte à l'injure, vous essayez plus tard pour libérer la mémoire une chaîne littérale dans la vie

free(player); 

ainsi très probablement brouillage sérieusement les structures de données du gestionnaire de tas. Encore une fois, vous semblez être malchanceux de ne pas causer un crash immédiat, mais le code qui semble fonctionner comme prévu est l'une des pires possibilités de comportement indéfini de se manifester. Si ce n'était pas le cas avant tous les paris, ils le seraient maintenant complètement.

---

Je suis désolé si cela semble condamner, il est vraiment pas signifiait cette façon, mais vous avez certainement et sérieusement fucked up celui-ci. Pour conclure:

1. Vous avez besoin d'un bon livre C++. Maintenant. Here est une liste de bons livres assemblés par des programmeurs C sur Stack Overflow. (Je suis un programmeur C++ par cœur, donc je ne vais pas commenter leur jugement, mais K & R est certainement un bon choix.)

2. Vous devez initialiser tous les pointeurs immédiatement, soit avec l'adresse d'un existant objet valide, ou avec l'adresse d'un morceau de mémoire alloué pour contenir un objet du bon type, ou avec NULL (que vous pouvez facilement vérifier pour plus tard). En particulier, vous ne devez pas essayer de lire ou d'écrire sur une partie de la mémoire qui n'a pas été allouée (dynamiquement sur le tas ou automatiquement sur la pile).

3. Vous devez free() toute la mémoire qui a été obtenue en appelant malloc()exactement une fois. Vous ne devez pas essayer de free() toute autre mémoire.

---

Je suis sûr qu'il ya plus à ce code, mais je vais arrêter ici. Et ai-je mentionné que vous avez besoin d'un bon livre C? Parce que vous faites.

Answer 2

player n'a pas besoin d'être libéré car il n'a jamais été malloc 'd, c'est simplement une variable de pile locale. player->name doit être libéré car il a été alloué dynamiquement.

int main() 
{ 
    // Declares local variable which is a pointer to a Player struct 
    // but doesn't actually point to a Player because it wasn't initialised 
    struct Player *player; 

    // Allocates space for name (in an odd way...) 
    player->name = malloc(sizeof(player->name)*256); 

    // At this point, player->name is a pointer to a dynamically allocated array of size 256*4 

    // Replaces the name field of player with a string literal 
    player->name = "John"; 


    // At this point, the pointer returned by malloc is essentially lost... 
    printf(player->name); 

    // ?!?! 
    free(player); 

    printf("\n\n\n"); 
    system("PAUSE"); 
    return 0; 
} 

Je suppose que vous vouliez faire quelque chose comme ceci:

int main() { 
    struct Player player; 
    player.name = malloc(256); 
    // Populate the allocated memory somehow... 

    printf("%s", player.name); 
    free(player.name); 
} 

Answer 3

Vous devez free() tout ce que vous malloc() et vous devez malloc() tout ce qui n'est pas affecté au moment de la compilation.

Alors:

Vous devez malloc player et vous devez libérer player->name

Answer 4

Ok, donc votre lecteur variable est un pointeur, que vous ne l'avez pas initialisé, et porte donc sur l'emplacement de mémoire vive.

Vous devez d'abord allouer la mémoire pour player comme vous l'avez fait pour player->name, puis alocate pour player-> name. La mémoire allouée avec malloc() doit être libérée avec free().

Jetez un oeil à this et this.

Answer 5

C'est un code horrible. Pourquoi? Tout d'abord, vous allouez de la mémoire pour player-> name. malloc renvoie le pointeur sur la mémoire allouée. A l'étape suivante, vous perdez cette valeur de pointeur parce que réaffectez player-> name pour pointer vers une chaîne "John" statique. Peut-être que vous voulez utiliser les fonctions strdup ou sprintf?

La grosse erreur est aussi d'utiliser un pointeur non initialisé vers la structure du joueur. Essayez d'imaginer qu'il peut pointer vers un emplacement de mémoire aléatoire. C'est donc une bonne idée d'allouer de la mémoire à votre structure avec l'aide de malloc. Ou n'utilisez pas de pointeur pour structurer et utiliser une variable de structure réelle.