0

コマンドラインからargvを使用してchar入力を受け入れるプログラムがあります。strcpy を使用して入力 argv[1] を、メモリが割り当てられている structptr というポインタ (構造体から structptr->words に移動) にコピーします。次に、ポインタ structptr が指すメモリから、割り当てられたメモリを指す words と呼ばれる別のポインタを 1 文字ずつコピーします。1文字をコピーした後、その要素[c]を印刷して、正しくコピーされたことを確認します(これはあります)。次に、すべての文字のコピーを終了し、結果を char ポインターに返しますが、何らかの理由で空白/null です。文字をコピーするたびに、前の要素が正しいかどうかを確認しましたが、それらはもう表示されません([c-2]、[c-1]、[c])。これが私のコードです：

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

struct StructHolder {
char *words;
};
typedef struct StructHolder Holder;

char *GetCharacters(Holder *ptr){
int i=0;
char *words=malloc(sizeof(char));
for(i;i<strlen(ptr->words);i++){
 words[i]=ptr->words[i];
 words=realloc(words,sizeof(char)+i);
}
words[strlen(ptr->words)]='\0';
return words;
}

int main(int argc, char **argv){

Holder *structptr=malloc(sizeof(Holder));
structptr->words=malloc(strlen(argv[1]));
strcpy(structptr->words, argv[1]);
char *charptr;
charptr=(GetCharacters(structptr));
printf("%s\n", charptr);

return 0;
4

3 に答える 3

2

最初はこれが問題だと思いました：

char *words=malloc(sizeof(char))1 バイト (1 文字のサイズ) を割り当てています。あなたはおそらく意味しましたchar *words = malloc(strlen(ptr->words)+1);-あなたはおそらくptrをnullチェックしたいと思うでしょう、そしてそれは安全のためにメンバーです。

それから私は見ましたrealloc。あなたの再割り当ては常に1文字短いです。i = 0 の場合、1 バイトを割り当ててからループに入り、インクリメントiして、再割り当てされた配列の末尾 (インデックス 1) の後ろに char 1 を配置します。

また、あなたstrcpyのメインはホルダーにメモリを割り当てていません。

于 2013-09-17T23:37:16.960 に答える
0

この2行で、

structptr->words=malloc(strlen(argv[1]));
strcpy(structptr->words, argv[1]);

nul ターミネータを保持するには、サイズに 1 を追加する必要があります。strlen(argv[1])する必要がありますstrlen(argv[1])+1。

ループでも同じことが起こっていると思います。1 だけ大きくする必要があります。またsizeof(char)、定義上は常に 1 であるため、次のようになります。

 ...
 words=realloc(words,i+2);
}
words=realloc(words,i+2); // one more time to make room for the '\0'
words[strlen(ptr->words)]='\0';
于 2013-09-17T23:50:29.527 に答える
0

参考：あなたの説明はについて語ってstructptrいますが、あなたのコードはstruct StructHolderandを使用していますHolder。

このコードは災害です:

char *GetCharacters(Holder *ptr){
    int i=0;
    char *words=malloc(sizeof(char));
    for(i;i<strlen(ptr->words);i++){
        words[i]=ptr->words[i];
        words=realloc(words,sizeof(char)+i);
    }
    words[strlen(ptr->words)]='\0';
    return words;
}

そのはず：

char *GetCharacters(const Holder *ptr)
{
    char *words = malloc(strlen(ptr->words) + 1);
    if (words != 0)
        strcpy(words, ptr->words);
    return words;
}

あるいは：

char *GetCharacters(const Holder *ptr)
{
    return strdup(ptr->words);
}

そして、それらはすべて、構造体型を渡すことが理にかなっていることを受け入れています。代わりに単に渡さない明確な理由はありませんconst char *words。

「災害」を分析する（および引数の型を無視する）：

char *GetCharacters(Holder *ptr){
    int i=0;

ここまではOKですが、構造を変更するつもりはないので、const Holder *ptr引数になる可能性があります。

    char *words=malloc(sizeof(char));

1 バイトを割り当てるのはコストがかかります — を呼び出すよりもコストがかかりstrlen()ます。これは良いスタートではありませんが、それ自体は間違っていません。ただし、メモリ割り当てが成功したかどうかはチェックしません。それは間違いです。

    for(i;i<strlen(ptr->words);i++){

最初のi;用語は明らかに奇妙です。あなたは書くことができますfor (i = 0; ...（そしておそらくの定義でイニシャライザを省略しますi、またはあなたは書くことができますfor (int i = 0; ....

そのようなループで繰り返し使用strlen()することも悪いニュースです。以下を使用する必要があります。

    int len = strlen(ptr->words);
    for (i = 0; i < len; i++)

次：

        words[i]=ptr->words[i];

この割り当ては問題ではありません。

        words=realloc(words,sizeof(char)+i);

このrealloc()割り当ては問題です。null ポインターが返された場合は、以前に割り当てられたメモリへの唯一の参照が失われています。したがって、戻り値を個別に保存してテストし、成功した場合にのみ割り当てる必要があります。

        void *space = realloc(words, i + 2);  // When i = 0, allocate 2 bytes.
        if (space == 0)
            break;
        words = space;

これはより良い/より安全です。完全にきれいではありません。早期終了を行うにはbreak;に置き換えたほうがよいかもしれません。{ free(words); return 0; }しかし、一度に 1 バイトを割り当てるこのビジネス全体は、正しい方法ではありません。割り当てるスペースの量を計算してから、一度にすべて割り当てる必要があります。

    }
    words[strlen(ptr->words)]='\0';

iの代わりに を使用することで、長さの再計算を避けることができstrlen(ptr->words)ます。が実行された場合、これは正しいという副次的な利点がありますif (space == 0) break;。

    return words;
}

この機能の残りの部分は OK です。

私は分析に時間を費やしていませんmain()。ただし、問題がないわけではありません。

于 2013-09-18T00:03:31.567 に答える