Valgrind, um Speicherlecks zu beheben

Question

Ich habe ein Programm, das ich in einer Textdatei von 30.000 Zahlen lesen, die Leerzeichen getrennt sind (5 pro Zeile). Wenn ich das Programm laufe, funktioniert es manchmal, aber manchmal nicht (seg faults), was meiner Meinung nach bedeutet, dass ich Speicherlecks habe.

Ich habe folgende:

int main(int argc, char const *argv[]) 
{ 

    char input[256]; 
    char buffer[30000]; 
    char **nums = NULL; 
    nums = malloc(sizeof(char) * 30000); 

    char *token; 
    int counter = 0; 
    FILE *fp; 
    fp = fopen("textfile.txt","r"); 

    fgets(input,sizeof(input),stdin); 

    while (fgets(buffer,sizeof(buffer),fp) != NULL) 
    { 
     token = strtok(buffer," "); 
     nums[counter] = malloc(sizeof(char) * 50); 

     while (token != NULL) 
     { 
      if (strlen(token) > 1) 
      { 
       strcpy(nums[counter],token); 
       counter++; 
      } 
      token = strtok(NULL," "); 
     } 
    } 

    for (int x = 0; x < 30000;x++) 
    { 
     free(nums[x]); 
    } 

    free(nums); 
    fclose(fp); 

    return 0; 
} 

I valgrind leite zu debuggen, um zu versuchen, aber ich Probleme habe die Ausgabe zu lesen. Kann mir jemand sagen, wo ich falsch liege?

==24368== Use of uninitialised value of size 8 
==24368== at 0x4C2588C: strcpy (mc_replace_strmem.c:311) 
==24368== by 0x400820: main (in /home/a.out) 
==24368== 
==24368== Invalid write of size 1 
==24368== at 0x4C2588C: strcpy (mc_replace_strmem.c:311) 
==24368== by 0x400820: main (in /home/a.out) 
==24368== Address 0x0 is not stack'd, malloc'd or (recently) free'd 
==24368== 

Answer 1

Wenn Sie mit Debugsymbolen kompilieren, normalerweise mit -g, dann wird valgrind Datei- und Zeilennummern in seine Ausgabe aufnehmen, was es viel einfacher zu verstehen macht.

---

@Jean-FrançoisFabre already found one malloc problem, enthält Ihre while-Schleife mehr.

while (fgets(buffer,sizeof(buffer),fp) != NULL) 
{ 
    token = strtok(buffer," "); 
    nums[counter] = malloc(sizeof(char) * 50); 

Sie Zuteilen immer Speicher für nums[counter], aber ...

while (token != NULL) 
    { 
     if (strlen(token) > 1) 
     { 
      strcpy(nums[counter],token); 
      counter++; 

Sie manchmal nur counter erhöhen. Sie werden viel Speicher verlieren.

Sie sind auch token kopieren, die auf 29.998 Bytes werden könnte, in nums[counter], die nur 50.

 } 
     token = strtok(NULL," "); 
    } 
} 

---

Stattdessen ist, nums[counter] zuteilen, wie es gebraucht wird und auf die richtige Größe.

while (fgets(buffer,sizeof(buffer),fp) != NULL) 
{ 
    token = strtok(buffer," "); 

    while (token != NULL) 
    { 
     if (strlen(token) > 1) 
     { 
      nums[counter] = malloc(sizeof(char) * (strlen(token) + 1)); 
      strcpy(nums[counter],token); 
      counter++; 
     } 
     token = strtok(NULL," "); 
    } 
} 

Oder, wenn es Ihnen nichts ausmacht, POSIX-Funktionen zu verwenden, verwenden Sie strdup.

nums[counter] = strdup(token); 

Und, wie andere haben in den Kommentaren darauf hingewiesen, ist es fraglich, warum Sie Zahlen als Strings sind zu speichern. Konvertiere sie sofort in Zahlen mit strtol oder strtod und speichere das. Es ist einfacher und verbraucht viel weniger Speicher.

long nums[30000]; 

... 

char *end; 
nums[counter] = strtol(token, &end, 10); 
if(end != '\0') { 
    fprintf(stderr, "Couldn't understand %s\n", token); 
} 

end ist ein Zeiger auf die Stelle, wo auf tokenstrtol Parsen gestoppt. Überprüfen, ob es ein Nullbyte ist, muss token nur Ziffern enthalten. Wie streng Sie mit Ihrer Konvertierung sein wollen, liegt ganz bei Ihnen.

Answer 2

nums = malloc(sizeof(char) * 30000); 

werden 30000 Zeiger auf Zahlen nicht halten. Die Größe ist zu niedrig, sollte sein:

nums = malloc(sizeof(char*) * 30000); 

Abgesehen, anstatt das zu tun:

token = strtok(buffer," "); 
nums[counter] = malloc(sizeof(char) * 50); 

Sie tun sollten:

token = strtok(buffer," "); 
nums[counter] = malloc(strlen(token)+1); 

, so dass Sie die richtige Menge an Speicher für jede zuteilen Token (nicht zu viel, aber nicht zu wenig), und beachte, dass sizeof(char) immer 1 ist, also weglassen.