Pourquoi est-ce que j'obtiens une double erreur gratuite avec realloc() ?

StackOverflow https://stackoverflow.com/questions/1145

  •  08-06-2019
  •  | 
  •  

Question

J'ai essayé d'écrire une fonction de remplacement de chaîne en C, qui fonctionne sur un char *, qui a été alloué à l'aide de malloc().C'est un peu différent dans le sens où il recherchera et remplacera les chaînes, plutôt que les caractères de la chaîne de départ.

C'est trivial à faire si les chaînes de recherche et de remplacement ont la même longueur (ou si la chaîne de remplacement est plus courte que la chaîne de recherche), car j'ai suffisamment d'espace alloué.Si j'essaie d'utiliser realloc(), j'obtiens une erreur qui m'indique que je fais un double free - ce que je ne vois pas comment je suis, puisque j'utilise uniquement realloc().

Peut-être qu'un petit code aidera :

void strrep(char *input, char *search, char *replace) {
    int searchLen = strlen(search);
    int replaceLen = strlen(replace);
    int delta = replaceLen - searchLen;
    char *find = input;

    while (find = strstr(find, search)) {

        if (delta > 0) {
            realloc(input, strlen(input) + delta);
            find = strstr(input, search);            
        }

        memmove(find + replaceLen, find + searchLen, strlen(input) - (find - input));
        memmove(find, replace, replaceLen);
    }
}

Le programme fonctionne, jusqu'à ce que j'essaye de realloc() dans un cas où la chaîne remplacée sera plus longue que la chaîne initiale.(Cela fonctionne toujours, cela génère simplement des erreurs ainsi que le résultat).

Si cela peut vous aider, le code d'appel ressemble à :

#include <stdio.h>
#include <string.h>
#include <stdlib.h>

void strrep(char *input, char *search, char *replace);

int main(void) {
    char *input = malloc(81);

    while ((fgets(input, 81, stdin)) != NULL) {
        strrep(input, "Noel", "Christmas");
    }
}
Était-ce utile?

La solution

En règle générale, vous devriez jamais effectuez une libération ou une réallocation sur un tampon fourni par l'utilisateur.Vous ne savez pas où l'utilisateur a alloué l'espace (dans votre module, dans une autre DLL), vous ne pouvez donc utiliser aucune des fonctions d'allocation sur un tampon utilisateur.

À condition que vous ne puissiez plus effectuer aucune réallocation au sein de votre fonction, vous devez modifier un peu son comportement, comme effectuer un seul remplacement, afin que l'utilisateur puisse calculer la longueur maximale de la chaîne résultante et vous fournir un tampon suffisamment long pour celui-ci. le remplacement doit avoir lieu.

Ensuite, vous pouvez créer une autre fonction pour effectuer les remplacements multiples, mais vous devrez allouer tout l'espace pour la chaîne résultante et copier la chaîne d'entrée utilisateur.Ensuite, vous devez fournir un moyen de supprimer la chaîne que vous avez allouée.

Résultant en:

void  strrep(char *input, char *search, char *replace);
char* strrepm(char *input, char *search, char *replace);
void  strrepmfree(char *input);

Autres conseils

Tout d'abord, désolé, je suis en retard à la fête.Ceci est ma première réponse stackoverflow.:)

Comme cela a été souligné, lorsque realloc() est appelé, vous pouvez potentiellement modifier le pointeur vers la mémoire en cours de réallocation.Lorsque cela se produit, l'argument "string" devient invalide.Même si vous le réaffectez, la modification devient hors de portée une fois la fonction terminée.

Pour répondre à l'OP, realloc() renvoie un pointeur vers la mémoire nouvellement réaffectée.La valeur de retour doit être stockée quelque part.En général, vous feriez ceci :

data *foo = malloc(SIZE * sizeof(data));
data *bar = realloc(foo, NEWSIZE * sizeof(data));

/* Test bar for safety before blowing away foo */
if (bar != NULL)
{
   foo = bar;
   bar = NULL;
}
else
{
   fprintf(stderr, "Crap. Memory error.\n");
   free(foo);
   exit(-1);
}

Comme TyBoer le souligne, vous ne pouvez pas modifier la valeur du pointeur transmis en entrée à cette fonction.Vous pouvez attribuer ce que vous voulez, mais la modification sera hors de portée à la fin de la fonction.Dans le bloc suivant, « input » peut ou non être un pointeur invalide une fois la fonction terminée :

void foobar(char *input, int newlength)
{
   /* Here, I ignore my own advice to save space. Check your return values! */
   input = realloc(input, newlength * sizeof(char));
}

Mark essaie de contourner ce problème en renvoyant le nouveau pointeur comme sortie de la fonction.Si vous faites cela, il incombe à l'appelant de ne plus jamais utiliser le pointeur qu'il a utilisé pour la saisie.Si cela correspond à la valeur de retour, alors vous avez deux pointeurs vers le même endroit et vous n'avez qu'à appeler free() sur l'un d'eux.S'ils ne correspondent pas, le pointeur d'entrée pointe désormais vers la mémoire qui peut ou non appartenir au processus.Le déréférencer pourrait provoquer une erreur de segmentation.

Vous pouvez utiliser un double pointeur pour l'entrée, comme ceci :

void foobar(char **input, int newlength)
{
   *input = realloc(*input, newlength * sizeof(char));
}

Si l'appelant a un double du pointeur d'entrée quelque part, ce double peut toujours être invalide maintenant.

Je pense que la solution la plus propre ici est d'éviter d'utiliser realloc() lorsque vous essayez de modifier l'entrée de l'appelant de fonction.Il suffit de malloc() un nouveau tampon, de le renvoyer et de laisser l'appelant décider de libérer ou non l'ancien texte.Cela présente l’avantage supplémentaire de permettre à l’appelant de conserver la chaîne d’origine !

Juste une photo dans le noir car je ne l'ai pas encore essayé, mais lorsque vous réaffectez, il renvoie le pointeur un peu comme malloc.Étant donné que la réallocation peut déplacer le pointeur si nécessaire, vous utilisez probablement un pointeur non valide si vous ne procédez pas comme suit :

input = realloc(input, strlen(input) + delta);

Quelqu'un d'autre s'est excusé d'être en retard à la fête - il y a deux mois et demi.Eh bien, je passe beaucoup de temps à faire de l'archéologie logicielle.

Je suis intéressé par le fait que personne n'a commenté explicitement la fuite de mémoire dans la conception originale ou l'erreur un par un.Et c'est l'observation de la fuite de mémoire qui me dit exactement pourquoi vous obtenez l'erreur de double libération (car, pour être précis, vous libérez la même mémoire plusieurs fois - et vous le faites après avoir piétiné la mémoire déjà libérée).

Avant de procéder à l’analyse, je serai d’accord avec ceux qui disent que votre interface est loin d’être excellente ;cependant, si vous avez traité les problèmes de fuite de mémoire/piétinement et documenté l'exigence « de la mémoire doit être allouée », cela pourrait être « OK ».

Quels sont les problèmes?Eh bien, vous transmettez un tampon à realloc(), et realloc() vous renvoie un nouveau pointeur vers la zone que vous devez utiliser - et vous ignorez cette valeur de retour.Par conséquent, realloc() a probablement libéré la mémoire d'origine, puis vous lui transmettez à nouveau le même pointeur, et il se plaint que vous libérez la même mémoire deux fois parce que vous lui transmettez à nouveau la valeur d'origine.Non seulement cela perd de la mémoire, mais cela signifie que vous continuez à utiliser l'espace d'origine - et la photo de John Downey dans le noir souligne que vous utilisez mal realloc(), mais ne souligne pas à quel point vous le faites.Il existe également une erreur ponctuelle car vous n'allouez pas suffisamment d'espace pour le NUL '\0' qui termine la chaîne.

La fuite de mémoire se produit parce que vous ne fournissez pas de mécanisme pour informer l'appelant de la dernière valeur de la chaîne.Parce que vous avez continué à piétiner la chaîne d'origine ainsi que l'espace qui la suit, il semble que le code ait fonctionné, mais si votre code appelant libère de l'espace, il obtiendra également une erreur de double libération, ou il pourrait obtenir un core dump ou équivalent car les informations de contrôle de la mémoire sont complètement brouillées.

Votre code ne protège pas non plus contre une croissance indéfinie : pensez à remplacer « Noel » par « Joyeux Noel ».À chaque fois, vous ajouteriez 7 caractères, mais vous trouveriez un autre Noel dans le texte remplacé, vous le développeriez, et ainsi de suite.Mon correctif (ci-dessous) ne résout pas ce problème – la solution simple consiste probablement à vérifier si la chaîne de recherche apparaît dans la chaîne de remplacement ;une alternative consiste à ignorer la chaîne de remplacement et à poursuivre la recherche après celle-ci.Le second présente des problèmes de codage non triviaux à résoudre.

Donc, ma révision suggérée de votre fonction appelée est la suivante :

char *strrep(char *input, char *search, char *replace) {
    int searchLen = strlen(search);
    int replaceLen = strlen(replace);
    int delta = replaceLen - searchLen;
    char *find = input;

    while ((find = strstr(find, search)) != 0) {
        if (delta > 0) {
            input = realloc(input, strlen(input) + delta + 1);
            find = strstr(input, search);            
        }

        memmove(find + replaceLen, find + searchLen, strlen(input) + 1 - (find - input));
        memmove(find, replace, replaceLen);
    }

    return(input);
}

Ce code ne détecte pas les erreurs d'allocation de mémoire - et plante probablement (mais sinon, perd de la mémoire) si realloc() échoue.Voir le livre « Writing Solid Code » de Steve Maguire pour une discussion approfondie sur les problèmes de gestion de la mémoire.

Remarque, essayez de modifier votre code pour vous débarrasser des codes d'échappement HTML.

Eh bien, même si cela fait un moment que je n'ai pas utilisé C/C++, la réallocation qui augmente ne réutilise la valeur du pointeur de mémoire que s'il y a de la place en mémoire après votre bloc d'origine.

Par exemple, considérez ceci :

(xxxxxxxxxxxx..........)

Si votre pointeur pointe vers le premier x, et .signifie un emplacement mémoire libre, et vous augmentez la taille de la mémoire indiquée par votre variable de 5 octets, cela réussira.Il s'agit bien sûr d'un exemple simplifié puisque les blocs sont arrondis à une certaine taille pour l'alignement, mais quand même.

Cependant, si vous essayez par la suite de l'augmenter de 10 octets supplémentaires et qu'il n'y en a que 5 disponibles, il devra déplacer le bloc en mémoire et mettre à jour votre pointeur.

Cependant, dans votre exemple, vous transmettez à la fonction un pointeur vers le caractère, pas un pointeur vers votre variable, et ainsi, même si la fonction strrep en interne peut ajuster la variable utilisée, il s'agit d'une variable locale vers la fonction strrep et votre code d'appel conservera la valeur de la variable de pointeur d'origine.

Cette valeur de pointeur a cependant été libérée.

Dans votre cas, la saisie est le coupable.

Cependant, je ferais une autre suggestion.Dans votre cas, cela ressemble à saisir La variable est effectivement entrée, et si c'est le cas, elle ne doit pas du tout être modifiée.

J'essaierais donc de trouver une autre façon de faire ce que tu veux faire, sans changer saisir, car de tels effets secondaires peuvent être difficiles à détecter.

Cela semble fonctionner ;

char *strrep(char *string, const char *search, const char *replace) {
    char *p = strstr(string, search);

    if (p) {
        int occurrence = p - string;
        int stringlength = strlen(string);
        int searchlength = strlen(search);
        int replacelength = strlen(replace);

        if (replacelength > searchlength) {
            string = (char *) realloc(string, strlen(string) 
                + replacelength - searchlength + 1);
        }

        if (replacelength != searchlength) {
            memmove(string + occurrence + replacelength, 
                        string + occurrence + searchlength, 
                        stringlength - occurrence - searchlength + 1);
        }

        strncpy(string + occurrence, replace, replacelength);
    }

    return string;
}

Soupir, est-il possible de poster du code sans que ce soit nul ?

realloc est étrange, compliqué et ne doit être utilisé que lorsqu'il s'agit de beaucoup de mémoire plusieurs fois par seconde.c'est à dire.- où cela rend votre code plus rapide.

J'ai vu du code où

realloc(bytes, smallerSize);

a été utilisé et travaillé pour redimensionner le tampon, le rendant plus petit.Cela a fonctionné environ un million de fois, puis, pour une raison quelconque, realloc a décidé que même si vous raccourcissiez le tampon, cela vous donnerait une belle nouvelle copie.Donc, vous vous écrasez dans un endroit aléatoire 1/2 seconde après que les mauvaises choses se soient produites.

Utilisez toujours la valeur de retour de realloc.

Mes conseils rapides.

Au lieu de:
void strrep(char *input, char *search, char *replace)
essayer:
void strrep(char *&input, char *search, char *replace)

et que dans le corps :
input = realloc(input, strlen(input) + delta);

Lisez généralement sur la transmission d'arguments de fonction en tant que valeurs/référence et description realloc() :).

Licencié sous: CC-BY-SA avec attribution
Non affilié à StackOverflow
scroll top