2012-02-07 15 views
2

私はC#で初めて、ポイントで作業するC言語の新しい人です。ループ内のセグメンテーションフォールト

私は将来にmalloc()realloc()free()で動作します。この機能を持っている:私が呼ぶとき

char ** split(char * delimiter, char * input) { 

     int i = 0; 
     int size = sizeof(char *); 
     char ** tokens; 
     char * token; 
     char * state; 
     tokens = (char **) malloc(size); 

     if(tokens == NULL) { 
      printf("Allocation failed."); 
      return; 
     } 

     for(token = strtok_r(input, delimiter, &state); 
      token != NULL; 
      token = strtok_r(NULL, delimiter, &state), 
      i++, size *= i) { 

      tokens = (char **) realloc(tokens, size); 

      if(tokens == NULL) { 
       printf("Realloc failed."); 
       return; 
      } 

      tokens[i] = state; 
     } 

     return tokens; 
} 

を:

char * IPNumber = "127.0.01"; 
char * delimiter = "."; 
char ** parts = split(delimiter, IPNumber); 

それはsegmentation faultを与えます。

realloc()関数の2番目の引数で使用するサイズ値を取得(計算)する方法を探しています。前もって感謝します。

+3

あなたがしようとしていることを説明すると、より良い回答が得られます。 – IanGilham

+0

どのように関数を呼び出していますか? – cnicutar

+0

IanGilham、cnicutar私の編集を参照してください –

答えて

0

完全書き換え。投稿された元のコードにはいくつかの問題があります。

  • 再割り当てサイズの計算が正しくありません。
  • 文字列定数のstrtok_rへの引き渡しが無効です。最初の引数を変更するので、文字列リテラルが渡されたときにアクセス違反が発生する可能性があります。
  • 結果配列にトークンの割り当ては、位置1の代わりに0
  • 割り当てではなく、トークンの状態変数を使用する(おそらくすべてではない、所望の結果におそらく未定義の動作)で開始します。
  • 返された配列内のトークンの数を呼び出し元が知る方法はありません。
  • reallocへの呼び出しに失敗しても元のポインタは解放されないため、リークする可能性があります。

変更を記述しようとするのではなく、他のものと同じパターンを使用して、可能な最大トークン数に基づいて1回の割り当てでよりクリーンな実装を示します。

char ** split(char * delimiter, char * input) { 
    int size; 
    int maxsize; 
    char ** tokens; 
    char * token; 
    char * state; 

    // compute max possible tokens, which is half the input length. 
    // Add 1 for the case of odd strlen result and another +1 for 
    // a NULL entry on the end 
    maxsize = strlen(input)/2 + 2; 
    tokens = (char**)malloc(maxsize * sizeof(char*)); 
    if(tokens == NULL) { 
     printf("Allocation failed."); 
     return NULL; 
    } 

    size = 0; 
    for(token = strtok_r(input, delimiter, &state); 
     token != NULL; 
     token = strtok_r(NULL, delimiter, &state)) { 

     tokens[size++] = token; 
    } 

    assert(size < maxsize); 
    // Put a NULL in the last entry so the caller knows how many entries 
    // otherwise some integer value would need to be returned as an output 
    // parameter. 
    tokens[size] = NULL; 

    // NOTE: could use realloc from maxsize down to size if desired 

    return tokens; 
} 

使用方法は次のようになります。あなたのmalloc /のcallocの

char * IPNumber = strdup("127.0.01"); 
char * delimiter = "."; 
char ** parts = split(delimiter, IPNumber); 
int i; 

if (parts) { 
    for (i = 0; parts[i] != NULL; i++) 
     printf("%s\n", parts[i]); 

    free(parts); 
    } 

free(IPNumber); 
+0

あなたの返信ありがとうございます。私は代入 'tokens [i] = token;の前に' i ++ 'をループからインクリメントして、あなたが言うように計算しようとしました。しかし、それは私の問題を解決しなかった、私は同じエラーが発生します。 –

+0

@TheMask:おそらく、ループのインクリメントを動かす際にはっきりしていなかったでしょう。その場合、ループの最下部にある必要があります。割り当ての前にインクリメントすると、配列の最初の位置がスキップされ、メモリが上書きされます。編集して明確にしようと思います。 –

2

サイズは(あなたが配列のcount!の成長になり意図したカウントで乗算)

間違っている:関数に文字列定数を渡すことを避けるために strdupの使用を注意してください

最初の項目について:i = 0、size = sizeof(char *);第二項目を I = 1、サイズ=さはsizeof(文字)/ *それは二つの要素のためには小さすぎる*/

char ** split(char * delimiter, char * input) { 

     unsigned size , used; 
     char ** array = NULL; 
     char * token; 
     char * state; 

     size = used = 0; 
     for(token=strtok_r(input, delimiter, &state); token; token=strtok_r(NULL, delimiter, &state)) { 

      if (used+1 >= size) { 
         size = size ? 2*size: 4; 
         array = realloc(array, size * sizeof *array); 

         if (!array) { printf("Realloc failed."); return NULL ; /*leak here*/ } 
      } 

      array[used++] = state; 
     } 
     /* NOTE: need a way to communicate the number of elements back to the caller */ 
     if (array) array[used] = NULL; 

     return array; 
} 

UPDATE:ここでテストドライバーは、私は、[OK]を

int main(void) 
{ 
char stuff[] = "this is the stuff"; 
char **ppp; 
unsigned idx; 

ppp = split(" " , stuff); 
for (idx = 0; ppp && ppp[idx]; idx++) { 
     fprintf(stdout, "%u: %s\n", idx, ppp[idx]); 
     } 
return 0; 
} 
+0

あなたのコンピュータで動作しましたか? –

+0

はい、動作します(つまり、クラッシュしません)。最初の要素をスキップし、最後に空の文字列を指す余分な要素を追加します。しかし、それはクラッシュしません。 BTW:あなたがそれを呼び出す方法、*入力は読み取り専用文字列 "127.0.0.1"を指しています。それは動作しません、strtok()は書き込み可能な文字列を求めています。 – wildplasser

+0

あなたのプラットフォームとコンパイラは何ですか?それは私のために働いていません:ubuntu 10.4、gcc 4.4.3。 –

2

あります

char ** split(char * delimiter, char * input) { 
     int i; 
     char ** tokens; 
     char * token; 
     char * state; 

     tokens = (char **) malloc(sizeof(char *) * (2)); 

     if(tokens == NULL) { 
      printf("Allocation failed."); 
      return NULL; 
     } 

     tokens[0]=(char *)1; /* one element populated */ 
     tokens[1]=NULL; /* no tokens */ 

     for(i=1, token = strtok_r(input, delimiter, &state); 
      token != NULL; 
      token = strtok_r(NULL, delimiter, &state), 
      i++) { 

      /* grow array by one element - originally made with 2 */ 
      { 
       char **new =(char **) realloc(tokens, (i+2) * sizeof(char *)); 

       if(new == NULL) { 
        printf("Realloc failed."); 
        free(tokens); 
        return NULL; 
       } 
       else 
       { 
        tokens = new; 
        tokens[i+1] = NULL; /* initialize new entry */ 
       } 
      } 

      tokens[i] = token; 
      tokens[0] = (char *)i; 
     } 

     return tokens; 
} 

int main(void) 
{ 
    char str[] = "129.128.0.1"; 
    char delim[] = "."; 
    char **ret; 

    ret = split(delim, str); 

    printf("tokens = %d\n", (int)ret[0]); 
    printf("tokens[1] = %s\n", ret[1]); 
    printf("tokens[2] = %s\n", ret[2]); 
    printf("tokens[3] = %s\n", ret[3]); 
    printf("tokens[4] = %s\n", ret[4]); 
    printf("tokens[5] = %s\n", ret[5]); 
} 
    のものが含まれます。推測何を意図して文字列の配列を返すようにしました
  1. は明示的な値を返しますが、ごみは返しません。
  2. realloc機能が変更されました。各ループ中に1つの要素だけ配列を拡大します。
  3. メモリリークを修正しました。
  4. strtok_rによって返された値を保存します。プライベート内部状態変数は返しません。
  5. 配列は、それが可能なので、それは、配列のエントリゼロは、あなたが巨大な文字列
を処理しているわけではないオーバーフローべきでない限り、サイズ、ある
  • NULLに初期化されることを確認する必要があり、その後1大きいです
  • +0

    $ ./strings.exe トークンは= 4つの トークン[1] = 129個の トークン[2] = 128個の トークン[3] = 0 トークン[4] = 1つの トークン[5] =(NULL) – Fred

    0

    私は修正するために、物事を指摘しようとしてますが、次のようにだけではなく、それを書き直しました:

    char **split(char *delim, char *input) 
    { 
        char *save; /* saved state for strtok_r */ 
        char **tmp, /* temporary result from realloc (for error handling) */ 
         **res; /* result - NULL-terminated array of tokens */ 
        int i,  /* index of current/last token */ 
         count; /* number of elements in res (including NULL) */ 
    
        /* Allocate first element for res */ 
        if (!(res = malloc(sizeof(res[0])))) { 
        /* return NULL if malloc() fails */ 
        fprintf(stderr,"split(): malloc() failed\n"); 
        return NULL; 
        } 
    
        /* res[0] = first token, or NULL */ 
        res[0] = strtok_r(input,delim,&save); 
    
        /* if it was a token, grab the rest. Last one will be the NULL 
        * returned from strtok_r() */ 
        if (res[0]) 
        i = 0; 
        count = 1; 
        do { 
         /* Resize res, for next token */ 
         /* use a temporary pointer for realloc()'s result, so that 
         * we can check for failure without losing the old pointer */ 
         if (tmp = realloc(res, sizeof(res[0]) * ++count)) 
         res = tmp; 
         else { 
         /* if realloc() fails, free res and return NULL */ 
         free(res); 
         fprintf(stderr,"split(): realloc() failed.\n"); 
         return NULL; 
         } 
         /* get next token, or NULL */ 
         res[++i] = strtok_r(NULL,delim,&save); 
        } while (res[i]); /* done when last item was NULL */ 
    
        return res; 
    } 
    

    だから、reallocのためのサイズは、要素のサイズを掛け、必要な要素の数です。

    上記のバージョンのコードは、NULLで終了する配列を返します。もう1つのアプローチは、何らかの形で配列の要素の数を返すことです(int *またはsize_t *引数のように)。いずれにしても、呼び出し元が結果配列の終わりがどこにあるかを知る方法が必要です。また、このためstrtok_r()を使用

    は別の漁獲量を追加します:元input文字列がそのまま残さないです。したがって、この(または元の)関数を使用するときには、元の文字列を保持する必要がないときに使用するか、元の文字列を最初に複製する必要があります。

    関連する問題