2010-11-18 41 views
2

Voici mon code:corruption Heap lors de la suppression d'une chaîne

std::string readString() 
{ 
    int strLen = Read<int>(); 
    char* rawString = new char[strLen]; 
    Read(rawString, strLen); 
    rawString[strLen] = '\0'; 
    std::string retVal(rawString); 
    delete [] rawString; 
    return retVal; 
} 

La première ligne lit la longueur de la chaîne.
La deuxième ligne crée un nouveau tableau char (c-string) avec la longueur de la chaîne
La troisième ligne lit la chaîne (elle la lit dans un fichier)
La 4ème ligne ajoute une valeur NULL à la fin.
La 5ème ligne crée une chaîne std :: hors de la chaîne de caractères.
La 6ème ligne supprime la chaîne de caractères (HEAP CORRUPTION HAPPENS HERE)
La 7ème ligne renvoie la chaîne, mais elle n'atteint jamais ce point en raison d'une erreur.

Sur la ligne 6, j'obtiens une erreur de corruption de tas: CRT a détecté que l'application a écrit en mémoire après la fin du tampon de tas.

Ma question peut être évidente, mais pourquoi suis-je obtenir une corruption de tas? Quand je crée une chaîne std ::, elle doit copier la chaîne, et je devrais pouvoir supprimer la chaîne de caractères en toute sécurité.

Actuellement, je soupçonne que std :: string essaie d'accéder à la chaîne de caractères après l'avoir supprimée.

Des idées?

+0

Vous avez 'delete []' dans votre code, donc votre code est mauvais. Utilisez un 'std :: vector' ou quelque chose, ou même lisez directement dans la chaîne. – GManNickG

+0

@GMan: N'a même pas vu votre commentaire avant que je posté: p C'est incroyable comment il simplifie le code aussi ... –

Répondre

4

Change:

char* rawString = new char[strLen]; 

à:

char* rawString = new char[strLen + 1]; 
+0

Merci. Je ne peux pas croire que j'ai oublié quelque chose comme ça. J'utilise C++ depuis un moment maintenant :). Merci à tous ceux qui ont posté de l'aide là-dessus aussi! – Brad

2

int strLen = Read<int>()probablement retourne que la longueur d'une chaîne terminée non nulle, et lorsque vous essayez d'écrire le \0 octet à la chaîne, vous rencontrez des problèmes de débordement de la mémoire tampon.

Vous devriez vérifier ce que strLen est, et le plus probable que vous devez soit affecter comme ceci:

char *rawString = new char[strlen+1]; 

Ou utilisez le constructeur surchargé de std::string(const char *, size_t n) comme ceci:

std::string retVal(rawString, strlen); 
8

Vous accédez passé les octets réservés pour votre chaîne. Vous avez réservé strLen caractères, mais mettez un \0 au caractère strLen. Comptant comme des tableaux C à partir de 0, le caractère strLen est à la position strLen + 1, donc vous placez une valeur en dehors de l'espace réservé pour la chaîne. Vous devez réserver strLen + 1 dans la deuxième ligne de votre main pour que votre code fonctionne.

1

Comme les tableaux sont 0-indexées dans C++, lorsque vous créez un tableau de taille strLen puis placez un 0 à la position strLen, vous êtes écrire que zéro après la fin du tableau que vous avez attribué.

0
rawString[strLen] = '\0'; 

Écrit le NUL à la fin de l'espace que vous avez alloué. Si strLen est 10, alors vous allouez de l'espace pour 10 caractères, lisez 10 caractères, et écrivez ce NUL à la position 11.Ooops

1

De nombreux conseils jusqu'à présent, mais aucun qui traite le problème de sécurité exception: comment vous débarrasser de cette fuite de mémoire potentiel?

Il existe deux façons d'éviter l'allocation avec new (et donc de faire face à une fuite de mémoire). La première est très simple et utilise une extension du compilateur connu sous le nom VLA Array Variable Length:

std::string readString() 
{ 
    int strLen = Read<int>(); 
    char rawString[strLen+1]; // VLA: the length is determined at runtime 
          // but the array is nonetheless on the stack 
    Read(rawString, strLen); 
    rawString[strLen] = '\0'; 

    std::string retVal(rawString); 
    return retVal; 
} 

L'autre est conforme à la norme: string a une mémoire tampon interne que vous pouvez accéder (grâce à GMan, data n'est pas la bonne méthode d'accès)

std::string readString() 
{ 
    int strLen = Read<int>(); 

    std::string retVal(strLen, '\0'); // no need to allocate extra space 

    Read(&retVal[0], strLen);  // &retVal[0] gives access to the buffer 

    return retVal; 
} 

Je crois que la dernière version est BEAUCOUP mieux. Il n'y a plus de copie impliquée :)

+0

Le premier est C++ non standard, en fait. Et le second ne donne que l'accès const. : S Vous voulez soit lire dans un 'vector' ou réserver la chaîne et lire dans' & retVal [0] ', ce qui crache un tampon contigu. Oh, et vous avez "extrêmement simplement". :) – GManNickG

+0

@GMan: merde, je pensais qu'il y avait deux versions de 'data'. J'ai précisé que le premier n'était pas standard, je précise que c'est une extension de compilateur (et une belle ...) –