Frage

So habe ich ein paar Funktionen, die mit einem string Typ arbeiten ich geschaffen habe. Einer von ihnen schafft einen dynamisch zugewiesenen Stachel. Der andere nimmt die Schnur, und erstreckt sich davon. Und die letzte befreit den String. . Hinweis: Die Funktionsnamen geändert werden, aber alle sind individuell definiert durch mich

string new = make("Hello, ");
adds(new, "everyone");
free(new);

Der obige Code funktioniert - es kompiliert und läuft gut. Der folgende Code funktioniert nicht - es kompiliert, läuft, und dann

string new = make("Hello, ");
adds(new, "everyone!");
free(new);

Der Unterschied zwischen dem Code ist, dass die Funktion adds() Zugabe noch 1 Zeichen (a !). Der Charakter fügt es macht keinen Unterschied - nur die Länge. Nur der Vollständigkeit halber, wird der folgende Code nicht:

string new = make("Hello, ");
adds(new, "everyone");
adds(new, "!");
free(new);

Merkwürdig ist, dass der folgende Code, die eine andere Funktion verwendet, addc() (die anstelle einer Zeichenfolge 1 Zeichen hinzufügt) funktioniert:

string new = make("Hello, ");
adds(new, "everyone");
addc(new, '!');
free(new);

Die folgenden, die auch die gleiche Sache tut, funktioniert:

string new = make("Hello, everyone!");
free(new);

Der Fehler, dass alle diejenigen, die nicht funktionieren geben, ist dies:

test(526) malloc: *** error for object 0x100130: double free
*** set a breakpoint in malloc_error_break to debug

(test sind die extrem beschreibenden Namen des Programms Ich habe dies in.)

Was die Funktion Interna, mein make() einen Anruf Anruf strlen() und zwei malloc() und einen Aufruf an memcpy(), mein adds() ist ein Aufruf an strlen(), ein Aufruf an realloc(), und ein Aufruf an memcpy(), und meine free() ist zwei Anrufe auf die Standardbibliothek free().

So gibt es irgendwelche Ideen, warum ich dies immer, oder muss ich brechen und einen Debugger verwenden? Ich bekomme es nur mit adds()es von über eine bestimmte Länge, und nicht mit addc()s.

Breaking down und Buchungscode für die Funktionen:

typedef struct _str {
  int _len;
  char *_str;
} *string;

string make(char *c)
{
    string s = malloc(sizeof(string));
    if(s == NULL) return NULL;
    s->_len = strlen(c);
    s->_str = malloc(s->_len + 1);
    if(s->_str == NULL) 
      {     
        free(s);
        return NULL;
      }
    memcpy(s->_str, c, s->_len);
    return s;
}

int adds(string s, char *c)
{
    int l = strlen(c);
    char *tmp;
    if(l <= 0) return -1;
    tmp = realloc(s->_str, s->_len + l + 1);
    if(!tmp) return 0;
    memcpy(s->_str + s->_len, c, l);
    s->_len += l;
    s->_str[s->_len] = 0;
    return s->_len;
}

void myfree(string s)
{
    if(s->_str) free(s->_str);
    free(s);
    s = NULL;
    return;
}
War es hilfreich?

Lösung

Eine Reihe von möglichen Problemen würde ich beheben:

1 / Ihr make() gefährlich ist, da es nicht ist über den Nullabschluss für die Zeichenfolge zu kopieren.

2 / Es macht auch wenig Sinn s zu setzen in NULL myfree(), da es ein übergebene Parameter ist und wird keine Auswirkung auf den tatsächlichen Parametern übergeben hat.

3 / Ich bin mir nicht sicher, warum Sie -1 zurück von adds(), wenn die zugegebenen Stringlänge 0 oder weniger ist. Erstens kann es nicht negativ sein. Zweitens scheint es durchaus plausibel, dass Sie einen leeren String hinzufügen könnte, die in nicht ändert die Zeichenfolge und Rückgabe des aktuellen String-Länge führen sollte. Ich würde nur eine Länge von -1 zurück, wenn es scheitert (das heißt realloc() funktionierte nicht) und sicherstellen, dass die alte Zeichenfolge beibehalten wird, wenn das passiert.

4 / Sie sind nicht die tmp Variable in s->_str Speicherung obwohl es ändern kann - es selten wieder reserviert Speicher an Ort und Stelle, wenn Sie die Größe sind zu erhöhen, obwohl es möglich ist, wenn sich der Anstieg klein genug ist, nach innen zu passen jeder zusätzlichen Platz durch malloc() zugeordnet. Reduzierung der Größe würde an Ort und Stelle an Sicherheit grenzender Wahrscheinlichkeit Neuzuweisung es sei denn, Ihre Implementierung von malloc() verschiedenen Pufferpools für unterschiedlich große Speicherblocks verwendet. Aber das ist nur eine Seite, da Sie nicht immer die Speicherauslastung mit diesem Code zu reduzieren.

5 / Ich denke, Ihr spezifische Problem hier ist, dass Sie nur sind die Zuteilung Platz für Zeichenfolge, die ein Zeiger auf die Struktur ist, nicht die Struktur selbst. Das bedeutet, wenn Sie die Zeichenfolge in setzen, können Sie die Speicher Arena sind korrumpiert.

Dies ist der Code, den ich geschrieben hätte (einschließlich beschreibender Variablennamen, aber das ist nur meine Vorliebe).

Ich habe geändert:

  • die Rückgabewerte von adds() um besser auf die Länge und Fehlerbedingungen zu reflektieren. Jetzt nur noch gibt sie -1 zurück, wenn es nicht (und die ursprüngliche Zeichenfolge ist unberührt) erweitern kann -. Anderer Rückgabewert ist die neue Stringlänge
  • die Rückkehr von myfree(), wenn Sie wirklich wollen, wollen die Zeichenfolge setzen mit so etwas wie „s = myfree (s)“ auf NULL.
  • die Kontrollen in myfree() für NULL Zeichenfolge, da Sie jetzt nie ohne einen zugewiesenen string einen zugewiesenen string->strChars haben.

Hier ist es (oder nicht :-) wie Sie sehen, passen:

/*================================*/
/* Structure for storing strings. */

typedef struct _string {
    int  strLen;     /* Length of string */
    char *strChars;  /* Pointer to null-terminated chars */
} *string;

/*=========================================*/
/* Make a string, based on a char pointer. */

string make (char *srcChars) {
    /* Get the structure memory. */

    string newStr = malloc (sizeof (struct _string));
    if (newStr == NULL)
        return NULL;

    /* Get the character array memory based on length, free the
       structure if this cannot be done. */

    newStr->strLen = strlen (srcChars);
    newStr->strChars = malloc (newStr->strLen + 1);
    if(newStr->strChars == NULL) {     
        free(newStr);
        return NULL;
    }

    /* Copy in string and return the address. */

    strcpy (newStr->strChars, srcChars);
    return newStr;
}

/*======================================================*/
/* Add a char pointer to the end of an existing string. */

int adds (string curStr, char *addChars) {
    char *tmpChars;

    /* If adding nothing, leave it alone and return current length. */

    int addLen = strlen (addChars);
    if (addLen == 0)
        return curStr->strLen;

    /* Allocate space for new string, return error if cannot be done,
       but leave current string alone in that case. */

    tmpChars = malloc (curStr->strLen + addLen + 1);
    if (tmpChars == NULL)
        return -1;

    /* Copy in old string, append new string. */

    strcpy (tmpChars, curStr->strChars);
    strcat (tmpChars, addChars);

    /* Free old string, use new string, adjust length. */

    free (curStr->strChars);
    curStr->strLen = strlen (tmpChars);
    curStr->strChars = tmpChars;

    /* Return new length. */

    return curStr->strLen;
}

/*================*/
/* Free a string. */

string myfree (string curStr) {
    /* Don't mess up if string is already NULL. */

    if (curStr != NULL) {
        /* Free chars and the string structure. */

        free (curStr->strChars);
        free (curStr);
    }

    /* Return NULL so user can store that in string, such as
       <s = myfree (s);> */

    return NULL;
}

Die einzige andere mögliche Verbesserung ich einen Puffer von Raum und das Ende des strChars sein sehen würde, könnte halten ein Niveau von Expansion zu ermöglichen, ohne malloc() aufrufen.

Das würde erfordern sowohl eine Pufferlänge und einen Stringlänge und Änderung des Codes nur mehr Platz zuzuweisen, wenn die kombinierte String-Länge und neue Zeichen Länge, die größer ist als die Pufferlänge ist.

Dies würde alle in der Funktion eingekapselt werden, so dass die API nicht ändern würden. Und wenn Sie jemals um Funktionen zu bieten, die Größe eines Strings zu reduzieren, müßten sie nicht entweder Speicher neu zuweisen, sie würden nur ihre Nutzung des Puffers reduzieren. Sie würden vermutlich eine compress() Funktion in diesem Fall müssen Strings reduzieren, die einen großen Puffer und kleine Schnur haben.

Andere Tipps

Der erste malloc in make sollte sein:

malloc (sizeof (struct _str));

Ansonsten sind Sie nur genug Platz für ein Zeiger Zuordnen struct _str.

 tmp = realloc(s->_str, s->_len + l + 1);

realloc kann einen neuen Zeiger auf den angeforderten Block zurück. Sie benötigen die folgende Codezeile hinzuzufügen:

 s->_str = tmp;

Der Grund, es nicht in einem Fall nicht abstürzen, aber tut nach einem weiteren Charakter Zugabe ist wahrscheinlich nur, weil, wie Speicher zugeordnet ist. Es ist wahrscheinlich eine Mindestzuteilung Delta (in diesem Fall von 16). Also, wenn Sie die ersten 8 Zeichen für den hallo Alloc, weist er tatsächlich 16. Wenn Sie hinzufügen, die jeder es nicht 16 so nicht überschreitet Sie den ursprünglichen Block zurück. Aber für 17 Zeichen, realloc gibt einen neuen Speicherpuffer.

Versuchen Sie, fügen Sie wie folgt

 tmp = realloc(s->_str, s->_len + l + 1);
 if (!tmp) return 0;
 if (tmp != s->_str) {
     printf("Block moved!\n"); // for debugging
     s->_str = tmp;
 }

In Funktion adds, übernehmen Sie, dass realloc ändert nicht die Adresse des Speicherblocks, die neu zugewiesen werden muss:

tmp = realloc(s->_str, s->_len + l + 1);
if(!tmp) return 0;
memcpy(s->_str + s->_len, c, l);

Während dies für kleine Umschichtungen wahr sein kann (weil Größen von Blöcken von Speichern Sie sind abgerundet erhalten in der Regel Zuführungen zu optimieren), das nicht wahr ist im Allgemeinen. Wenn realloc Sie einen neuen Zeiger zurückgibt, das Programm immer noch die alte verwendet, die das Problem verursacht:

memcpy(s->_str + s->_len, c, l);

Wahrscheinlich sollte nach dem Code, aber die doppelten frei bedeutet, dass Sie auf dem gleichen Zeiger zweimal kostenlos anrufen.

  1. Handelt es sich um 1 für die \ 0-Byte am Ende Strlen?
  2. Wenn Sie einen Zeiger freizugeben, setzen Sie Ihr Member-Variable so auf NULL, dass Sie nicht wieder frei zu tun (oder zu einem bekannten schlechten Zeigern wie 0xFFFFFFFF)

Warum "my free () sind zwei Anrufe auf die Standardbibliothek kostenlos ()." Warum rufst du zweimal frei? Sie sollten nur einmal anrufen müssen.

Bitte posten Sie Ihre fügt hinzu (); und free () Funktionen.

Lizenziert unter: CC-BY-SA mit Zuschreibung
Nicht verbunden mit StackOverflow
scroll top