2017-08-06 6 views
0

J'utilise valgrind pour essayer de réparer les nombreuses fuites de mémoire dans une tâche sur laquelle je travaille et cela me donne quelques erreurs de lecture/écriture/non-initialisées. I pense Je sais d'où il vient en fonction de la sortie de valgrind, mais je n'arrive pas à comprendre comment le réparer pour la vie de moi. Je suis très nouveau en C++ donc je suis probablement en train de faire quelque chose de totalement incorrect avec la façon dont j'alloue (et ensuite j'essaie d'accéder à la mémoire allouée incorrectement) la mémoire du tableau, mais je ne peux pas comprendre exactement serait.Valgrind: Écriture non valide de la taille 8 provenant du constructeur de copie

Voici les différentes sorties valgrind:

Invalid write of size 8 
==13371== at 0x4013F5: family::setFriends(char**) (family.cpp:62) 
==13371== by 0x4: family::family(family const&) (family.cpp:31) 
==13371== by 0x402358: hashtable::node::node(family const&) (hashtable.h:29) 
==13371== by 0x401E81: hashtable::insert(char const*, family const&) (hashtable.cpp:87) 
==13371== by 0x4018CD: familymgr::addFamily(family&) (familymgr.cpp:15) 
==13371== by 0x402779: main (housinghelper.cpp:86) 
==13371== Address 0x5ad1810 is 0 bytes inside a block of size 32 free'd 
==13371== at 0x4C2F650: operator delete[](void*) (vg_replace_malloc.c:621) 
==13371== by 0x4013B5: family::setFriends(char**) (family.cpp:60) 
==13371== by 0x4: family::family(family const&) (family.cpp:31) 
==13371== by 0x402358: hashtable::node::node(family const&) (hashtable.h:29) 
==13371== by 0x401E81: hashtable::insert(char const*, family const&) (hashtable.cpp:87) 
==13371== by 0x4018CD: familymgr::addFamily(family&) (familymgr.cpp:15) 
==13371== by 0x402779: main (housinghelper.cpp:86) 
==13371== Block was alloc'd at 
==13371== at 0x4C2E8BB: operator new[](unsigned long) (vg_replace_malloc.c:423) 
==13371== by 0x40120F: family::family(family const&) (family.cpp:29) 
==13371== by 0x402358: hashtable::node::node(family const&) (hashtable.h:29) 
==13371== by 0x401E81: hashtable::insert(char const*, family const&) (hashtable.cpp:87) 
==13371== by 0x4018CD: familymgr::addFamily(family&) (familymgr.cpp:15) 
==13371== by 0x402779: main (housinghelper.cpp:86) 

message de valeur non initialisée:

Use of uninitialised value of size 8 
==13371== at 0x401E98: hashtable::insert(char const*, family const&) (hashtable.cpp:90) 
==13371== by 0x4018CD: familymgr::addFamily(family&) (familymgr.cpp:15) 
==13371== by 0x402779: main (housinghelper.cpp:86) 
==13371== Uninitialised value was created by a stack allocation 
==13371== at 0x401882: familymgr::addFamily(family&) (familymgr.cpp:11) 
==13371== 
==13371== Use of uninitialised value of size 8 
==13371== at 0x401EB9: hashtable::insert(char const*, family const&) (hashtable.cpp:91) 
==13371== by 0x4018CD: familymgr::addFamily(family&) (familymgr.cpp:15) 
==13371== by 0x402779: main (housinghelper.cpp:86) 
==13371== Uninitialised value was created by a stack allocation 
==13371== at 0x401882: familymgr::addFamily(family&) (familymgr.cpp:11) 

'amis' est une variable membre de char ** déclaré dans le fichier d'en-tête comme si:

char **friends; 

Tout semble provenir du constructeur de la copie:

family::family(const family &fam) : ID(NULL), famName(NULL), 
friends(NULL){ 
    setID(fam.ID); 
    setFamName(fam.famName); 
    setMembers(fam.members); 
    setCapacity(fam.capacity); 
    setNumFriends(fam.numFriends); 

    setFriends(fam.friends); 
} 

Voici le constructeur principal, juste au cas où je n'allouent pas correctement la mémoire pour friends dès le départ-go:

family::family(char *ID, char *famName, int members) : 
    ID(NULL), 
    famName(NULL), 
    members(members), 
    numFriends(-1), 
    capacity(DEFAULT_CAPACITY) 
{ 
    friends = new char*[capacity]; 
    for (int i = 0; i < numFriends; i++) 
     friends[i] = NULL; 

    setID(ID); 
    setFamName(famName); 
} 

Et est ici la fonction setFriends qui est référencée:

void family::setFriends(char** friendIn){ 
friends = new char*[sizeof(friendIn[0])/numFriends]; 
if(friends!=NULL) 
    delete [] friends; 
for (int i = 0; i < capacity;i++){ 
    this->friends[i] = friendIn[i]; 
    } 
} 

bool familymgr::addFamily(family &inputFam) { 
    char fam[100]; 
    inputFam.getID(fam); 

    table->insert(fam, inputFam); 
} 

getID:

void family::getID(char *id) const { 
    strcpy(id, this->ID); 
} 

Qu'est-ce que je fais de mal ici pour produire toutes ces erreurs?

+0

Ces erreurs valgrind ne décrivent pas les fuites. Toujours se concentrer sur la première erreur en premier - est-ce le premier que vous avez posté? – aschepler

+0

@aschepler ouais mon erreur. J'ai dit des fuites parce que j'ai une tonne de fuite de mémoire, mais je suppose que les fuites de mémoire proviennent des erreurs que j'ai posté. – ThomasJazz

+0

Où est le code de 'addFamily'? – 1201ProgramAlarm

Répondre

0

L'écriture invalide provient de setFriends, où vous supprimez la mémoire allouée à friends puis écrivez-y. Vous devez faire une nouvelle allocation dans setFriends avant de copier dans les amis.

Les messages de valeur non initialisée proviennent de deux variables nommées ID dans le constructeur family: le paramètre et le membre de la classe. Lorsque vous appelez setID(ID), cela référence le membre de classe (avec une valeur de NULL), pas le paramètre ID. Renommez les paramètres pour avoir des noms différents des champs de la classe.

+0

A moins de malentendu, n'ai-je pas déjà fait une nouvelle allocation pour' friends' avec 'friends = new char * [sizeof (fam.friends [0])/numFriends];' dans le constructeur de copie? – ThomasJazz

+0

C'est dans le constructeur. Cette mémoire est supprimée (libérée) avec 'delete [] friends' dans' setFriends', et vous devez allouer de la mémoire avant de pouvoir l'utiliser à nouveau. – 1201ProgramAlarm

+0

@ThomasJazz 'delete [] friends' suivi immédiatement par déréférencement, c'est-à-dire:' this-> friends [i] = ... 'est une * recette * pour un comportement indéfini. En fait, c'est garanti. Lisez à propos de la durée de vie de l'objet dynamique dans votre texte. – WhozCraig

0

Votre méthode setFriends écrit à la mémoire que vous ne possédez pas:

void family::setFriends(char** friendIn){ 
    friends = new char*[sizeof(friendIn[0])/numFriends]; 
    if(friends!=NULL) // <-- This will always be true, since friends was 
         //  assigned a non-null value in the new[] 
         //  expression above 
     delete [] friends; // <-- Here you free the memory you just allocated 
    for (int i = 0; i < capacity;i++){ 
     // At this point, friends is no longer pointing to a valid object 
     // so you are trying to write to memory you don't own 
     this->friends[i] = friendIn[i]; 
    } 
} 

Vous devez inverser le chèque de NULL et l'expression new[]. En outre, l'utilisation de sizeof(friendIn[0])/numFriends n'a aucun sens. sizeof(friendIn[0] sera toujours 4 ou 8, en fonction de la quantité de votre plate-forme. Je devine que devrait juste être capacity:

void family::setFriends(char** friendIn){ 
    if(friends != nullptr) { 
     delete [] friends; 
    } 
    friends = new char*[capacity]; 
    for (int i = 0; i < capacity;i++){ 
     this->friends[i] = friendIn[i]; 
    } 
} 

Cela fonctionne lorsqu'il est appelé à partir du constructeur de copie, mais gardez à l'esprit que vous êtes toujours faire une copie superficielle de friends.Les deux objets family pointent vers les mêmes chaînes, et si vous les insérez par la suite, tous les objets qui pointent dessus ne fonctionneront pas. Vous devriez vraiment faire friends un std::vector<std::string>> au lieu de faire toute cette gestion de la mémoire manuelle. Ensuite, vous pouvez rendre setFriends beaucoup plus simple et beaucoup plus sûr:

void family::setFriends(std::vector<std::string>> friendIn) { 
    friends = std::move(friendIn); 
}