1

動的メモリとコピー コンストラクターを処理する単純な割り当てについて、私の教授は単純な割り当てを割り当ててくれましたが、2 回目の発生時にエラーが発生しdelete []ます。

ヘッダファイル:

class Stream {
    int len;
    char *hold;
    char* newmem(int);
public:
    Stream ();
    Stream (int);
    Stream(const char *);
    ~Stream ( );
    void operator=(const Stream &);
    Stream(const Stream &);
    friend void show(Stream);
    void operator<<(const char*);
};

それはかなり単純なはずです。ここに私のコードがあります:

#include <iostream>
#include <new>
#include <cstring>
using namespace std;
#include "stream.h"

char* Stream::newmem(int x) {
    char * tmp;
    try {
        tmp = new char[x];
    }
    catch(std::bad_alloc) {
        tmp = NULL;
    }
    if(tmp)
        cout << "newmem:  " << (void *) tmp << endl;
    return tmp;
}

Stream::Stream ( ) {
    len = 1000;
    hold = newmem(len);
    if (hold)
        strcpy (hold, "");
}

Stream::Stream(int n) {
    len = n;
    hold = newmem(len);
    if (hold)
        strcpy (hold,"");
}

Stream::Stream(const char * dat) {
    len = strlen(dat) +1;
    hold = newmem(len);
    if (hold)
        strcpy(hold,dat);
}

Stream::Stream(const Stream &from) {
    cout << "in the copy constructor, allocating new memory..." << endl;
    cout << "original pointer address is: " << (void *) from.hold << endl;
    cin.get( );

    len=from.len;
    hold=newmem(len +1);
    cout << "new pointer address is: " << (void *) hold << endl;
    cin.get( );

    if(hold)
        strcpy (hold,from.hold);
}

Stream::~Stream ( ) {
    cout << "destruct:  " << (void *) hold << endl;
    cin.get( );
    if (hold)
        delete [] hold;
}

void Stream::operator= (const Stream &from) {
    if(hold)
        delete [ ] hold;
    len = from.len;
    hold=newmem(len +1);
    if (hold)
        strcpy(hold,from.hold);
}

void show (Stream prt) {
    cout << "String is: " << prt.hold << endl << "Length is: " << prt.len << endl;
}

void Stream::operator<< (const char *data) {
    int dlen = strlen(data);
    for (int i=0 ; i<=len && i<=dlen ; i++) {
        hold[i] = data[i];
    }
}

int main( ) {
   char data[ ] = "Growing up it all seems so one-sided;"
                  "Opinions all provided;"
                  "The future pre-decided;"
                  "Detached and subdivided;"
                  "In the mass production zone!"
                  "-Neil Peart- \"Subdivisions\"";

   Stream x1, x2(25), x3;
   x1 << data;
   x2 << data;
   show(x1);
   show(x2);

   x3 = x2;
   show(x3);

   return 0;
}

そして私の出力/エラー:

コピーコンストラクターで、新しいメモリを割り当てています...
元のポインタ アドレス: 0x804c008

新しいポインタ アドレス: 0x804c808

文字列は次のとおりです: 成長すると、すべてが一方的なものに見えます;意見はすべて提供されます;未来は事前に決定されています;切り離されて分割されています;大量生産ゾーンで!-ニール・パート-分割」
長さ: 1000
破壊: 0x804c808


コピーコンストラクターで、新しいメモリを割り当てています...
元のポインタ アドレス: 0x804c3f8

新しいポインタ アドレス: 0x804c808

String is: 成長すると、すべてがそう見える
長さ: 25
破壊: 0x804c808

*** glibc が検出されました *** a.out: free(): 無効なポインタ: 0x0804c808 ***
4

2 に答える 2

4

の for ループには、operator<<2つのoff-by-one エラーがあります。

for (int i=0 ; i<=len

を許可しますが、有効なインデックスはi==lenのみです。だから、あなたは最後から1つ書くことができます。hold0..(len-1)

第二に、thiton が指摘したように、\0スペースがあってもターミネータをコピーしません。

正しい実装は次のようになります。

void Stream::operator<< (const char *data) {
    int source_len = strlen(data);
    int copy_len = min(source_len, len-1); // allow for terminator

    for (int i=0; i<copy_len; i++) {
        hold[i] = data[i];
    }
    hold[copy_len] = '\0';
}

単純に を使用する方がよいでしょうstrncpy。

ハーフ オープン (または最後を 1 つ超える) 範囲を使用するイディオムは、直接の配列インデックスだけでなく、C++ イテレータでも標準であることに注意してください。したがって、常に表示されることを期待する必要があります

for (i=0; i<n; ++i) {

また

for (i = begin; i != end; ++i) {

また、通常、あなたのような閉範囲のループは、さらなる調査が必要な匂いとして扱う必要があります。

于 2013-03-20T21:15:21.553 に答える
1

最初にちょっとした自助アドバイス: メモリ アクセス エラーを検出するための最も重要なツールはvalgrind. プログラムで実行すると、割り当てられていない、または初期化されていないメモリにアクセスしようとするたびに警告が表示されます。知識に代わるものではありませんが、次善の策です。

あなたが得るものとは異なる出力が得られますが、エラーはここで相互作用しているようです:

  1. のoperator<<範囲チェックで 1 つずれのエラーがあります。1 バイト書きすぎます ( hold[len])。
  2. operator<<終端の null バイトを書き込むことはありません。どちらのエラーも によって呼び出されx2 << dataます。
  3. コピー コンストラクターが から文字列をコピーしようとすると、x2終端strcpyの null バイトが見つからず、両方とも の末尾から読み取られ、末尾をx2.hold超えて書き込まれますx3.hold。後者は無制限の破損の可能性があり、おそらくエラーを引き起こしました。

C 文字列を扱うときはいつでも、非常に確実に終端処理を正しく行ってください。修正版は次のとおりです。

void Stream::operator<< (const char *data) {
    int dlen = strlen(data);
    hold[len-1] = 0;
    for (int i=0 ; i < len-1 && i <= dlen ; i++) {
         hold[i] = data[i];
    }
}

または、std ライブラリを使用します。

void Stream::operator<< (const char *data) {
    strncpy(hold, data, len);
    hold[len-1] = 0;
}
于 2013-03-20T21:15:38.087 に答える