2010-12-05 12 views
0

GWW(ユーザー)の助けを借りてこのコードを思いついたので、今度はchar **を解放できません。ここで再割り当て後に2d配列を解放する問題

は、(それだけで、入力ファイルを読み込み、画面上に名前を表示します)私のコードです:

EDIT:

/* deallocate2D 
corresponding function to dynamically deallocate 2-dimensional array using 
* malloc. 
* accepts a char** as the "array" to be allocated, and the number of rows. 
* as with all dynamic memory allocation, failure to free malloc'ed memory 
* will result in memory leaks 
*/ 
void deallocate2D(char** array, int nrows) { 

    /* deallocate each row */ 
    int i; 
    for (i = 0; i < nrows; i++) { 
     free(array[i]); 
    } 

    /* deallocate array of pointers */ 
    free(array); 
} 

int readInputFile(FILE *fp, char **file_images) { 
    num_lines = 0; 
    int s = 10; 
    char line[MAX_LENGTH]; 
    char **final_filenames; 

    while (fgets(line, sizeof line, fp) != NULL) /* read a line */ { 
     if (line[0] != '\n') { 
      if (num_lines >= s) { 
       s += 100; 
       if ((file_images = (char**) realloc(file_images, s * sizeof (char*))) == NULL) { 
        printf("Error reallocating space for 2d array: %s\n", strerror(errno)); 
        return -1; 
       } 
      } 
      if ((file_images[num_lines] = malloc(MAX_LENGTH * sizeof (char))) == NULL) { 
       printf("Error allocating space for 2d array: %s\n", strerror(errno)); 
       return -1; 
      } 

      strncpy(file_images[num_lines], line, MAX_LENGTH); 
      if (file_images[num_lines] == NULL) { 
       printf("Strncpy failed: %s\n", strerror(errno)); 
       return -1; 
      } 
      printf("name of file %d is: %s \n", num_lines, file_images[num_lines]); 
      num_lines++; 
     } 
    } 
    printf("Num_lines: %d\n",num_lines); 
    //realloc to number of lines in the file, to avoid wasting memory 
    if ((final_filenames = realloc(file_images, num_lines * sizeof (char*))) == NULL) { 
     printf("Error reallocating space for 2d array: %s\n", strerror(errno)); 
     return -1; 
    } else { 
     file_images = final_filenames; 
     deallocate2D(final_filenames, num_lines); 
    } 
    return 0; 
    //don't forget to free lines 2d array! (here or at the end of the code) 
} 

int main(int argc, char *argv[]) { 
    //pixel* image; 
    char **images_filenames; 

    //check parameters 
    if (argc < 4) { 
     printf("Incorrect usage.\nPlease use \"./invert input_filename.ppm charWidth charHeight \"\n"); 
     return -1; 
    } 

    printf("Opening input file [%s]\n", argv[1]); 
    FILE *fpin = fopen(argv[1], "r"); 
    if (fpin == NULL) { 
     printf("Could not open input file\n"); 
     return -1; 
    } 
    if ((images_filenames = ((char**) malloc(10 * sizeof (char*)))) == NULL) { 
     printf("Error allocating initial space for 2d array: %s\n", strerror(errno)); 
     return -1; 
    } 

    if (readInputFile(fpin, images_filenames) == -1) { 
     printf("Error reading image filenames from input\n"); 
     return -1; 
    } 

    fclose(fpin); 
    printf("###########\n"); 

    deallocate2D(images_filenames, num_lines); 

    printf("Done!\n"); 
    return 0; 
} 

、なぜ私はできません、私は理解していませんfinal_filenames配列を解放してimages_filenamesを解放してください。* glibcが検出されました ./main:ダブルフリーまたは破損(!prev):0x0986d228 * *。

とにかく、このコードで何か誤りがあると思われる場合は、動作しますが、それを指摘してください。

答えて

3

の問題は、あなたがすでに解放されている可能性があり、ポインタを解放している、とあなたが使用しているどのくらいのスペースがわからない(一般的に)最も最近に割り当てられた領域へのポインタを持っていないということですあなたは正確に記憶を解放することはできません。 main()では、あなたが持っている:

char **images_filenames;      

[...]         

if ((images_filenames = ((char**) malloc(10 * sizeof (char*)))) == NULL) { 

[...] 

if (readInputFile(fpin, images_filenames) == -1) { 

[...] 

deallocate2D(images_filenames, num_lines); 

あなたは10個の文字ポインタを割り当て、その後、readInputFile()関数にその配列を渡します。その機能の中には、配列を再割り当てするコードがありますが、メインプログラムがその新しいアドレスが何であるかを知る方法は提供していません。あなたが行う方法は、変更したいものにポインタを渡すか、関数が変更された値を返すようにすることです(あるいは、パラメータの代わりにグローバル変数を使うようなsordidのプラクティスに頼っていますが、 )。

だから、あなたが必要です

if (readInputFile(fpin, &images_filenames) == -1) { 

そしてreadInputFile()機能では、変更の全体の多く必要とする - トリプルポインタ引数に対処するための大きなもの、その後、問題をコーディングの様々な:

int readInputFile(FILE *fp, char ***ppp_files) 
{ 
    num_lines = 0; 
    int s = 10; 
    char line[MAX_LENGTH]; 
    char **file_images = *ppp_files; 
    char **final_filenames; 

更新

は、私は、これは単にそれを宣言していない、num_linesを初期化していることに気づきませんでした。したがって、num_linesはある種のグローバル変数でなければなりません。以下の解説のいくつかは、これを可能にするために調整する必要があります。


これまでのところ、変更は(ほとんど)簡単です。私たちは 'char **'へのポインタを得ているので、トリプルポインタの引数です。次のコードを単純化するには、古い名前(file_images)でパラメータのローカルコピーを作成し、引数が指す値で初期化します。次のコードは、file_imagesと引き続き動作します。戻す前に引数が更新されていることを確認してください。

...除き、あなたはさんは10を= 'ことを前提としていますが、本当に、あなたは、主な機能は利用できた行数を教えてくれている必要があります。それは10行を割り振ったが、慎重な調査がなければ明らかではない。 main()プログラムに、あらかじめ割り当てられている行の数を指定する必要があります。これは、関数の追加引数です。また、main()プログラムが、deallocate2D()関数に、わからないために配列に含まれる行の数を伝えられないという問題に直面しています。コードのコンパイル方法は明確ではありません。ここにはローカル変数num_linesがありますが、宣言がないnum_linesにはmain()の変数があります。ローカル変数は、グローバル変数をマスクします。

while (fgets(line, sizeof line, fp) != NULL) { 
     if (line[0] != '\n') { 
      if (num_lines >= s) { 
       s += 100; 

多数の行を追加することをお勧めします。それは再配分の費用を「償却」する。

   if ((file_images = (char**) realloc(file_images, s * sizeof (char*))) == NULL) 

使用した手法には特定の問題があります。 ピュアコードスタイル:ラインが埋め込まれた割り当てでifが含まれており、それがあまりにも長くなったときに、条件の前に割り当てを分割:

   file_images = (char**) realloc(file_images, s * sizeof (char*)); 
       if (file_images == NULL) 

は今すぐ左の微妙なバグがちょうどあります。 realloc()が失敗した場合どうなりますか。

file_imagesの値がnullであるため、メモリをリークしました。そのため、以前に指摘した内容を解放する方法がありません。 書くことはありません:

x = realloc(x, size); 

これは、失敗した場合にメモリリーク!したがって、次のものが必要です。一般的なルールとして

   char **new_space = realloc(file_images, s * sizeof (char*)); 
       if (new_space == NULL) 
       { 
        printf("Error reallocating space for 2d array: %s\n", 
          strerror(errno)); 
        *ppp_files = file_images; 
        return -1; 
       } 
      } 

を、エラーメッセージがstderrに印刷する必要があります。私はこれを修正していない。

最後の(nullでない)値file_imagesをメインプログラムの変数に慎重にコピーしました。同じサイズでも同じことをする(別のインターフェースを変更する)のが適切かもしれませんし、構造体を使って配列とその基底へのポインタをカプセル化することもできます。

 if ((file_images[num_lines] = malloc(MAX_LENGTH * sizeof (char))) == NULL) 
     { 
      printf("Error allocating space for 2d array: %s\n", strerror(errno)); 
      return -1;    
     } 

このエラーリターンは*ppp_files = file_images;を設定する必要があります。

このテストは奇妙です。 file_images[num_lines]はnullではないことがわかりますが、strncpy()はそれを変更しません。テストやエラー処理は必要ありません。

  printf("name of file %d is: %s \n", num_lines, file_images[num_lines]); 
      num_lines++; 
     }                
    } 
    printf("Num_lines: %d\n",num_lines); 

OK ...

//realloc to number of lines in the file, to avoid wasting memory 

ニースタッチ。それはそれほど価値がありません。 64ビットマシンであっても、せいぜい1キロビット以下しか無駄にしています。しかし、きちんとしていることに害はありません - 良い。

if ((final_filenames = realloc(file_images, num_lines * sizeof (char*))) == NULL) { 
     printf("Error reallocating space for 2d array: %s\n", strerror(errno)); 
     return -1; 

また、返品前に*ppp_files = file_images;を設定する必要があります。

} else { 
     file_images = final_filenames; 

これは、main()プログラムの値には影響しません。もう一度*ppp_files = file_images;にする必要があります。

 deallocate2D(final_filenames, num_lines); 

慎重に割り当てられたスペースをすべて割り当て解除しますか?結局それを使うつもりはないのですか?上記の代入はポインタ値をコピーしただけです。成功リターン時に、メモリが既に割り当て解除された - それは

} 
    return 0; 
    //don't forget to free lines 2d array! (here or at the end of the code) 
} 

このコメントは間違っている...メモリのコピーを作成しませんでした。


Lemme推測 - あなたは編集のために 'vim'や別の 'vi'派生物を使用しません。列1に機能の開始ブレースがある人は、ファイルを「]]」または「[[」を使用して次の関数または前の関数の先頭に前後にジャンプすることができるためです。それはうまく動作しないコードで作業しています。


これは始まりの診断です。ここでは、ファイル名の配列を中継する構造を使用した作業コードを示します。構造体からコピーされたローカル変数を使用してreadInputFile()関数の本体を残し、構造体が常に正しく更新されるようにしました。

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

enum { MAX_LENGTH = 512 }; 

typedef struct FileNameArray 
{ 
    size_t nfiles; /* Number of file names allocated and in use */ 
    size_t maxfiles; /* Number of entries allocated in array */ 
    char **files;  /* Array of file names */ 
} FileNameArray; 

static void deallocate2D(FileNameArray *names) 
{ 
    for (size_t i = 0; i < names->nfiles; i++) 
     free(names->files[i]); 
    free(names->files); 
    names->nfiles = 0; 
    names->files = 0; 
    names->maxfiles = 0; 
} 

static int readInputFile(FILE *fp, FileNameArray *names) 
{ 
    int num_lines = names->nfiles; 
    int max_lines = names->maxfiles; 
    char **file_names = names->files; 
    char line[MAX_LENGTH]; 
    char **final_filenames; 

    while (fgets(line, sizeof line, fp) != NULL) 
    { 
     if (line[0] != '\n') 
     { 
      /* Remove newline from end of file name */ 
      char *nl = strchr(line, '\n'); 
      if (nl != 0) 
       *nl = '\0'; 
      if (num_lines >= max_lines) 
      { 
       max_lines += 100; 
       char **space = realloc(file_names, max_lines * sizeof (char*)); 
       if (space == NULL) 
       { 
        fprintf(stderr, "Error reallocating space for 2d array: %s\n", 
          strerror(errno)); 
        return -1; 
       } 
       names->maxfiles = max_lines; 
       names->files = space; 
       file_names = space; 
      } 
      if ((file_names[num_lines] = malloc(strlen(line) + 1)) == NULL) 
      { 
       fprintf(stderr, "Error allocating space for 2d array: %s\n", 
         strerror(errno)); 
       return -1; 
      } 
      names->nfiles++; 
      strcpy(file_names[num_lines], line); 
      printf("name of file %d is: %s \n", num_lines, file_names[num_lines]); 
      num_lines++; 
     } 
    } 

    printf("Num_lines: %d\n", num_lines); 
    //realloc to number of lines in the file, to avoid wasting memory 
    if ((final_filenames = realloc(file_names, num_lines * sizeof (char*))) == NULL) 
    { 
     fprintf(stderr, "Error reallocating space for 2d array: %s\n", 
       strerror(errno)); 
     return -1; 
    } 
    names->maxfiles = num_lines; 
    names->files = final_filenames; 
    return 0; 
} 

int main(int argc, char *argv[]) 
{ 
    FileNameArray names = { 0, 0, 0 }; 

    //check parameters 
    if (argc < 4) 
    { 
     fprintf(stderr, "Usage: %s input_filename.ppm charWidth charHeight\n", 
       argv[0]); 
     return -1; 
    } 

    printf("Opening input file [%s]\n", argv[1]); 
    FILE *fpin = fopen(argv[1], "r"); 
    if (fpin == NULL) { 
     fprintf(stderr, "Could not open input file %s (%s)\n", 
       argv[1], strerror(errno)); 
     return -1; 
    } 

    if ((names.files = malloc(10 * sizeof (char*))) == NULL) 
    { 
     fprintf(stderr, "Error allocating initial space for 2d array: %s\n", 
       strerror(errno)); 
     return -1; 
    } 
    names.maxfiles = 10; 

    if (readInputFile(fpin, &names) == -1) 
    { 
     fprintf(stderr, "Error reading image filenames from input\n"); 
     return -1; 
    } 

    fclose(fpin); 
    printf("###########\n"); 

    deallocate2D(&names); 

    printf("Done!\n"); 
    return 0; 
} 
+0

私はあなたの努力に感謝し、私はCコンパイラを手に入れてすぐにあなたの提案を理解し、適用しようとします。 NetBeansやGeanyのようなIDEを使用しています。 – neverMind

+0

@neverMind:作業コードでは、メモリ割り当てコードを関数に組み込む必要がありました'int addFileName(FileNameArray * names、const char * line);'のようなものであり、 'readInputFile()'の中でそれを呼び出す。それで、その入力コードに混乱が少なくなるでしょう。そして、 'main()'には事前割り当てはありません。関数は単純に割り当てを上から処理します。 –

関連する問題