2


copyString と concatString の 2 つの関数を作成するように依頼され、両方を実装しましたが、得られた出力で、より良い方法で実行できると言われましたが、その方法については説明されませんでした。
今、私はどうすればもっとうまくやれるのか私を殺しているので、ここにコードがあります。提案を聞いて喜んでいます。

void copyString (char **strDst, const char *strSrc)
{
     char *strTmp = NULL;
     int length = strlen (src);
     if (*strDst== NULL) 
     {
        *strDst= malloc (length);   
     }
     else 
     {  
         if (strlen(*strDst) != length)
         {
             strTmp = *strDst;
         }
         *strDst= malloc (length);  
     }
     strcpy (*strDst, strSrc);  
     if (strTmp != NULL)    
         free (strTmp );    
 }

void concatString (char **strDst, const char *cat)
{
     int cat_length = strlen (cat);
     if (cat_length > 0) 
     {  
         *strDst= realloc (*strDst, strlen (*strDst) + cat_length); 
          strcat (*strDst, cat);
     }
}




void main(int argc, char *argv[])
{
    char *str = NULL;
    copyString(&str, "Hello World");
    puts(str);
    copyString(&str,str+6);
    puts(str);
    concatString(&str, " Pesron");
}

出力は次のようになります:
1.Hello World
2. World
3. World Person

ありがとう。

4

2 に答える 2

4

エラー:

strlennul ターミネータを除く長さを返すため、割り当てたすべてのサイズが小さすぎます。

が falseの場合if (strlen(*strDst) != length)(つまり、長さが等しい場合)、古いバッファがリークします。

reallocどちらもmalloc失敗する可能性があるため、それに対処するコードを記述できるはずです。

正しい使用方法reallocは次のとおりです。

char *newbuf = realloc(oldbuf, newsize);
if (newbuf == NULL) {
    // handle the error somehow, and note that oldbuf is still allocated
} else {
    oldbuf = newbuf;
}

「何らかの方法でエラーを処理する」には、2 つの関数のドキュメントに記載されている失敗時の動作に応じて、何をすべきかを決定する必要がある場合があります。それが言わないなら、そうすべきです。

(Picky)intは、文字列の長さを保持するのに十分な大きさの型であるとは限りません。を使用しますsize_t(ただし、符号なしの型の使用が厳密に禁止されている場合を除きます。その場合は がありssize_tます)。

改善できる点:

関数の最後ではなくstrTmp、すぐに文字列を解放することができます。[編集: はい、必要があります。ソースと宛先のオーバーラップを許可するべきではcopyStringないという要件があるようです。concatString個人的には、まだ少し違った書き方をします。]

if (strTmp != NULL) free (strTmp );null ポインターを使用して呼び出すことは有効であるため、in the test は冗長freeであり、そうしても効果はありません。

*strDst= malloc (length);ではどちらの場合も行いますcopyString。

main解放されないため、メモリ リークが発生しますstr。

main返すべきintではありませんvoid。

これが私がそれらを書く方法です：

呼び出しコードを変更してエラーをチェックすることはできないため、呼び出すことができる何かをそこにabort()記述する必要があります。この関数は、呼び出しが失敗しないという前提で書かれているため、おそらく最も悪い解決策です。putsmainabort()

関数が成功または失敗を示す値を返す場合は、呼び出し元にとっておそらくより良いでしょうが、既存の呼び出しコードによって制約されます。正直なところ、プログラミングを行うのはまったく非現実的な状況ではありません...

void concatString (char **strDst, const char *cat) {
    size_t dstlen = *strDst ? strlen(*strDst) : 0;
    char *buf = realloc(*strDst, dstlen + strlen(cat) + 1);
    if (!buf) {
        abort();
    }
    strcpy(buf + dstlen, cat);
    *strDst = buf;
}

void copyString (char **strDst, const char *strSrc) {
    char *buf = malloc(strlen(strSrc) + 1);
    if (!buf) {
        abort();
    }
    strcpy(buf, strSrc);
    free(*strDst);
    *strDst = buf;
}
于 2012-11-17T14:26:15.623 に答える
0

スティーブジェソップが彼の答えで言及していることに加えて、あなたの情報源に誤りはありませんが、欠けています：

  • 入力パラメータの検証
  • エラー値を介してエラーを返します（たとえば、関数の整数の戻りコードとしてではなく、void
于 2012-11-17T14:31:43.760 に答える