0

私は C 文字列を持っていて、'$' の最初の出現から切り取られるように短くしたいと考えています。これが私のコードです：

int char_search(char exp[], int s, char what) {
    int i, occurrence=-1;
    for (i=0; i < s && occurrence == -1; ++i)
        if (exp[i] == what)
            occurrence = i;

    return occurrence;
}

int shorten(char *exp, int maxlength, char *exp_new) {
    int l, i;
    l = char_search(exp, maxlength, '$');
    exp_new = (char *) malloc((l+1)*sizeof(char));
    exp_new[l] = '\0';
    for (i = 0; i<l; i++) 
        exp_new[i] = exp[i];

    return l;
}

問題は、exp_new ポインター アドレスを上書きし始め、最初の文字のみを実際の配列にコピーすることです。また、exp_new はなぜか NULL を返します。(文字列の長さは正しくないかもしれませんが、それですべてが台無しになることはありません。)

4

4 に答える 4

0

ここにいくつかの問題があります:

  1. 関数から値を返すには

    • 返されるもののインスタンスにアドレスを渡し、関数でアドレスを逆参照してデータを保存します。
    • 経由で関数から値を返しますreturn。

  2. 配列要素をアドレス指定する場合、負の値は必要なく、シンプルでintは十分な幅がない可能性があるため、 を使用しますsize_t。あなたの場合のように、-1使用も使用する必要がある場合ssize_t。このような型は両方とも、C/POSIX 標準によって、すべてのマシン メモリをアドレス指定するのに十分な幅があり、この可能なすべての配列要素を使用できることが保証されています。

  3. C では、 の結果をキャストしないでmalloc()/calloc()/realloc()ください。

  4. const定数を宣言します。これにより、コンパイラが最適化され、より良いコードを記述できるようになります。

  5. 失敗する可能性がある関数は常にテストしてください。これは、システム コールだけでなく、独自の関数にも当てはまります。

    次のエラー チェックがありません:

    • malloc()
    • char_search()

上記の点を考慮すると、コードは次のように変更される可能性があります。

 ssize_t char_search(const char * exp, const size_t s, const char what) 
 {
   ssize_t occurrence = -1;

   for (size_t i = 0; i < s && occurrence == -1; ++i)
   {
     if (exp[i] == what)
     {
       occurrence = i;
     }
   }

   return occurrence;
}

ssize_t shorten(const char * exp, const size_t maxlength, char ** pexp_new) 
{
  ssize_t l = char_search(exp, maxlength, '$');
  if ((NULL != pexp_new) && (-1 != l)) /* This way if passing in NULL 
                                      as 3rd argument you just get l. */
  {
    *pexp_new = malloc((l + 1) * sizeof **pexp_new);
    if (NULL = *pexp_new)
    {
      l = -1; /* Indicate failure. */
    }
    else
    {
      size_t i = 0;

      (*pexp_new)[l] = '\0';

      for (; i < l; ++i) 
      {
        (*pexp_new)[i] = exp[i];
      }
    }
  }

  return l;
}
于 2013-11-12T10:30:35.907 に答える
0

あなたのコードにはたくさんのバグがあります。

l = char_search(exp, maxlength, '$'); -1 にならないように結果を確認する必要があります。

(char *) malloc((l+1)*sizeof(char));malloc の結果を確認する必要があります。また、malloc の結果をキャストしないでください。

for (i = 0; i<l; i++)これはヌル終了をコピーしません。最初にヌル終了してからデータをコピーするのは少し奇妙です。memcpy(exp_new, exp, l);ループをその後で置き換えますexecute exp_new[l] = '\0';。

問題の真の原因は、あいまいな変数名です。あなた自身を含め、誰も何lを意味するのかを知ることはできません。それか何かを呼び出してstr_length、読者が「ああ、これは文字列の長さなので、null 終端は含まれていません」と簡単に判断できるようにします。さらにl、1一部のコード エディターでは と がまったく同じに見えるため、一般的に変数名の選択は不適切です。

また、両方の機能に対応expする必要があります。const char*

char *exp_newスタックにポインタを格納します。そのポインターは、関数の実行中にのみ存在します。割り当てられたメモリを呼び出し元に返す場合は、パラメータを に変更する必要がありますchar** exp_new。

ただし、一般に、malloc されたメモリへのポインターを返すことは、やや不適切で危険な行為です。メモリがどこかで解放されていることを確認してください。

于 2013-11-12T10:42:16.923 に答える
0

また、exp_new はなぜか NULL を返します。(文字列の長さは正しくないかもしれませんが、それですべてが台無しになることはありません。)

これは少し心配です。exp_new が NULL を「返す」という事実を却下しますが、これは何かが正しくないという主要な警告サインです。

私が期待する最後のことは、null ポインターへの書き込みが有用な結果を生成することです。

問題を適切に説明していますか？

exp_new を呼び出し元に返したい場合は、このようにはしません。char **として宣言し、ローカルに割り当てられた文字列を割り当てる必要があります

char *local_exp_new = malloc(...) ;
*exp_new = local_exp_new ;
于 2013-11-12T11:40:55.087 に答える