Fehler beim Freigeben eines 2D-Arrays von Malloc'd-Strings in C

Question

Ich weiß, dass dies eine Frage ist, die ziemlich häufig gestellt wurde, aber ich habe mehr als 10 geschlossene Fragen ohne Glück gelesen, da meine Lösung mit den von anderen vorgeschlagenen Lösungen übereinstimmt .

Ich schreibe meine eigene Shell als eine Lernübung und dabei malloc'ing eine Reihe von Strings, die als Argumente eines Programmaufrufs dienen. Mein malloc wird wie folgt durchgeführt, wo argcnt die Anzahl der Argumente ist gegeben + 2 den Standard argv Format anzupassen und das Array ermöglichen null beendet und damit von execvp verwendet werden:

char ** args; 
args = (char **) malloc(argcnt*sizeof(char *)); 
for(i = 0; i < argcnt; i++) { 
    args[i] = (char *) malloc(80*sizeof(char)); 
    printf("Made %d\n", i); 
} 

Ich befreie dann den gleichen Speicher wie follows:

for (i = 0; i < argcnt; i++) { 
    free(args[i]); 
    printf("Freed %d\n", i); 
} 
free(args); 

Das Programm kompiliert, schlägt aber bei der Freigabe von argv [1] zur Laufzeit fehl. Die Beispielausgabe das Programm ausgeführt wird und dann ls -a -l Aufruf lautet wie folgt:

jack@ubuntu:~/myshell$ ./myshell 
[30/03 23:34] # ls -a -l 
Argcnt: 4 
Made 0 
Made 1 
Made 2 
Made 3 
Arg[0]: ls 
Arg[1]: -a 
Arg[2]: -l 
Arg[3]: (null) 
Freed 0 
*** Error in `./myshell': free(): invalid pointer: 0x0000000001a84c43 *** 
Aborted (core dumped) 

ich damit für die letzten 2 Stunden Ringen haben und können nicht herausfinden, was falsch ist, so einen Einblick in das Problem wäre sehr geschätzt.

EDIT: Die Funktion zur Zeit meines Programm zu brechen ist:

void breakargs(char * cmd, char ** args) { 
    char * tok = malloc(strlen(cmd)*sizeof(char)); //maximum token size is full cmd 
    char * str = malloc(strlen(cmd)*sizeof(char)); 
    int i=1; 
    strcpy(str, cmd); //maintains integrity of cmd 

    args[0] = tok = strtok(str, " "); 
    while (tok != NULL) 
    { 
     args[i] = tok = strtok(NULL, " "); 
     i++; 
    } 
    args[i] = '\0'; 
    free(tok); 
} 

2. EDIT: Das Problem war meine Neuzuweisung der von meiner ursprünglichen malloc mit strtok gemacht Zeiger so, dass der ursprüngliche Zeigerverweis verloren. Darüber hinaus könnte meine Verwendung einer bestimmten Stringlänge für Argumente möglicherweise zu Problemen führen. Die Lösung bestand darin, nur malloc args [i] zu verwenden, als ich die Länge der Zeichenfolge kannte, die dort gespeichert werden musste, und die Zeichenfolge dann mit strcpy an die Speicherstelle zu verschieben, anstatt sie direkt zuzuweisen. Dies bewahrt die Integrität der Zeigerreferenz und ermöglicht, dass sie korrekt freigegeben wird.

Answer 1

strtok gibt einen Zeiger in die Zeichenfolge zurück, die anfänglich an strtok übergeben wurde, es ist nicht erforderlich, Speicherplatz für diesen Zeiger zuzuweisen.

Wenn Sie den Rückgabewert strtok einer Zeigervariablen zuweisen, überschreiben Sie den Zeigerwert dieser Variablen mit dem neuen Wert. Das heißt, wenn diese Zeigervariable auf Speicher zeigt, den Sie bereits zugewiesen haben, wird dieser Speicher "durchgesickert", da Sie keinen Zeiger mehr darauf haben.

Kurz gesagt - wenn Sie Speicher für eine Zeigervariable zuweisen, weisen Sie dieser Zeigervariablen keinen anderen Wert zu, es sei denn, Sie haben bereits den Speicher oder setzen den Zeigerwert an eine andere Stelle.

In Ihrer breakArgs Funktion gibt es eine Reihe von Fragen:

void breakargs(char * cmd, char ** args) { 
    char * tok = malloc(strlen(cmd)*sizeof(char)); //maximum token size is full cmd 

Sie brauchen keine Speicher für tok zuweisen, wie Sie es mit einem Wert von strtok

char * str = malloc(strlen(cmd)*sizeof(char)); 
    int i=1; 
    strcpy(str, cmd); //maintains integrity of cmd 

    args[0] = tok = strtok(str, " "); 

Don Zuweisung wird‘ t weise direkt args[0] zu, da dies den Zeiger auf den Speicher überschreibt, den du bereits zugewiesen hast. Ordnen Sie stattdessen tok, dann strcpy(args[0], tok);

while (tok != NULL) 
    { 
     args[i] = tok = strtok(NULL, " "); 

Weisen Sie nicht direkt in args[i] wie die Zeiger auf die bereits zugewiesenen Speicher zu überschreiben. Ordnen Sie stattdessen tok, dann strcpy(args[i], tok); Sie sollten auch überprüfen, ob tok NULL ist, bevor Sie strcpy von ihm.

 i++; 
    } 
    args[i] = '\0'; 

Weisen Sie nicht direkt in args [i], anstatt das Ende von strcpy(args[i], "") hinweisen könnte, so dass Sie eine leere Zeichenkette am Ende.

free(tok); 

tok hier nicht kostenlos tun, als tok soll von diesem Zeitpunkt NULL sein, wenn Sie zu free (str) benötigen.

} 

beachten Sie auch einige der anderen Kommentare über die Überprüfung, dass die Argumente, die Sie verarbeiten nicht überschreiten die Grenze Sie auf dem Speicher abgelegt haben (zum Beispiel 80 Zeichen pro Stück).

Answer 2

Das Problem ist nicht in dem Code, den Sie zeigen; Es ist in dem Code, den du nicht zeigst. Ohne die vollständige Kommentar Kette gelesen zu haben, ich vermute, dass Sie so etwas wie dies tun:

args[0] = "ls"; 
args[1] = "-a"; 
args[2] = "-l"; 
args[3] = NULL; 

und dann die args Array zu befreien versuchen. Vielleicht war dein Code nicht ganz so eklatant, aber das Endergebnis kann sehr ähnlich sein. Sie könnten beispielsweise eine Befehlszeile aufteilen, die in einen Puffer gelesen wurde, und die Zeiger möglicherweise in Abschnitte des Puffers in die Array-Elemente args kopieren. (. Update: Diese seems das eigentliche Problem sein)

Diese den zugewiesenen Speicher ausgelöscht hat (Sie haben es verloren - Sie haben Speicher verloren), und später versucht man, nicht zugewiesenen Speicher frei (Zeiger die nicht von malloc() oder einem seiner Freunde zurückgegeben wurden). Die erste solche free() 'oft funktioniert', aber völlig vermasselt die Kontrollen von malloc() et al, so dass die zweite free() schlägt fehl. Viele, aber nicht alle modernen Systeme haben eine malloc(), die viele solcher Missbrauch erkennt.

Sie benötigen Code mehr wie:

strcpy(args[0], "ls"); 
strcpy(args[1], "-a"); 
strcpy(args[2], "-l"); 
free(args[3]); 
args[3] = NULL; 

Beachten Sie, dass 80 Bytes pro String Zuteilung war eine grobe Überallokation für diese Saiten. OTOH, könnte es auch für andere (längere) Argumente völlig unzureichend sein.Sie sollten den Speicher für args wie Sie selbst reservieren; Sie sollten wahrscheinlich den Speicher für die einzelnen Elemente entsprechend dem, was das entsprechende Argument enthält, zuordnen. Es gibt eine (POSIX, aber nicht Standard C) Funktion, die strdup() genannt wird, die Sie vielleicht suchen - sie reserviert genug Platz für eine Kopie einer Zeichenkette und kopiert die Zeichenkette in den zugewiesenen Platz. Alternativ benötigen Sie die Zuordnungen möglicherweise überhaupt nicht.

Wenn es für Ihre Plattform verfügbar ist, sollten Sie valgrind verwenden, um Ihre Speicherbelegung zu validieren und Ihren Missbrauch zu identifizieren.