メモリリークを修正するためのValgrind

Question

私は、スペースで区切られた30,000の数字のテキストファイル（1行に5つ）で読み込んでいるプログラムを持っています。私はプログラムを実行すると、時にはそれが動作しますが、他の回は私はメモリリークを意味すると私は信じていない（セグフォルト）はありません。

私は、次のしている：

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; 
} 

私はデバッグしようとするvalgrindのを実行していますが、私は出力を読んで問題を抱えています。誰かが私がどこに間違っているか教えてもらえますか？

==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

デバッグシンボルを使用してコンパイルする場合、通常は-gで、valgrindはファイルと行番号を出力に含めて、理解しやすくします。

---

@Jean-FrançoisFabre already found one malloc problemあなたのwhileループにはさらに多くの情報が含まれています。

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

あなたはいつもあなたが唯一時々counterをインクリメント...

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

nums[counter]のためのメモリを割り当てますが、しています。あなたはたくさんの記憶を漏らすでしょう。

また、代わりに、ちょうど50

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

---

あるnums[counter]に、29998のバイトまで可能性がありtokenを、コピーすることが必要だとしてnums[counter]を割り当てて、適切なサイズにしています。

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," "); 
    } 
} 

POSIX機能を使用しても構わない場合は、strdupを使用してください。

nums[counter] = strdup(token); 

また、他の人がコメントに指摘しているように、数字を文字列として保存する理由は疑問です。 strtolまたはstrtodですぐに数値に変換して保存してください。それはより簡単であり、より少ないメモリを消費します。

long nums[30000]; 

... 

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

endstrtolはパース停止token上のスポットへのポインタです。それがヌルバイトであることを確認するには、tokenに数字だけを含める必要があります。どのような厳格なコンバージョンを望むかは、あなた次第です。

Answer 2

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

番号に30000個のポインタを保持しません。サイズが小さすぎる、次のようになります。

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

別に

、代わりにやって：あなたがすべき

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

を：

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

ますので、それぞれのメモリの適切な量を割り当てますトークン（それほど多くはないが、少なすぎず）、sizeof(char)は常に1であることに注意してください。