2011-09-18 197 views
2

在下面的代码中,我遇到了问题。它表现为bad_alloc异常,这是因为传递给reader的参数CompressedBufferReader是一个错误的字符串。从构造函数调用构造函数

class FileReader { 
    FILE *file; 
protected: 
    unsigned char *data; // local copy 
    long size; 
public: 
    FileReader(const char *filename); 
    ~FileReader(); 
    unsigned long getSize(); 
    unsigned char *getFileData(); 
}; 

class CompressedBufferReader { 
    unsigned char *buffer; 
    unsigned long len; 
public: 
    CompressedBufferReader(unsigned char *); 
    ~CompressedBufferReader(); 
    unsigned char *getBuffer(); 
    unsigned long getLength(); 
}; 
CompressedBufferReader::CompressedBufferReader(unsigned char *srcCompressed) { 
    len = 0; buffer = 0; 
    len = GetDecompressedBufferSize(srcCompressed); 
    buffer = new unsigned char[len]; if (!buffer) throw std::runtime_error("Cannot allocate!"); 
    WriteDecompressedBuffer(buffer, len, srcCompressed); 
} 
CompressedBufferReader::~CompressedBufferReader() { 
    delete[] buffer; 
} 
unsigned char *CompressedBufferReader::getBuffer() {return buffer;} 
unsigned long CompressedBufferReader::getLength() {return len;} 

// similar interface to FileReader. Does not inherit because it does not benefit from doing so. 
class CompressedFileReader { 
    CompressedBufferReader reader; 
public: 
    CompressedFileReader(const char *filename); 
    unsigned char *getFileData(); 
    unsigned long getSize(); 
}; 

CompressedFileReader::CompressedFileReader(const char *filename) : reader(FileReader(filename).getFileData()){} // this line is causing the problem 
unsigned char *CompressedFileReader::getFileData() { return reader.getBuffer(); } 
unsigned long CompressedFileReader::getSize() { return reader.getLength(); } 

更具体地讲,好像我创建匿名其数据内容可被传递到reader构造变得之前释放的的FileReader,一个CompressedBufferReader

的问题是,我不能写CompressedFileReader的构造函数的方式,让我正确实例化一个FileReader,因为我打算用CompressedBufferReader的构造,这意味着构造函数的身体之前,我必须调用它。一个catch-22的位。这个问题如何解决?

+1

我觉得很难说没有看到'FileReader'的实现是什么问题。但是作为一般性评论,您的代码非常“手动”,如果您使用标准容器和字符串,读取和维护可能会更加简单。 –

回答

2

我没有看到你使用构造函数的方式,这应该会导致bad_alloc(尽管代码看起来笨拙)的问题。让我们看看这是导致问题的行执行 -

CompressedFileReader::CompressedFileReader(const char *filename) : 
    reader(FileReader(filename).getFileData()){} 

以下步骤进行:

  1. 一个时间FileReader创建。它的构造函数调用filename,这是一个const char*
  2. 构造函数做未知的事情,因为我们没有它的代码。我认为它应该将文件读入分配的缓冲区,存储在FileReaderdata成员中。
  3. getFileData()被称为临时FileReader,返回data的值,我假设这是一个unsigned char *
  4. reader,这是一个CompressedBufferReader,使用unsigned char *构造。
  5. 时空FileReader被销毁。

所以,问题不在于结构的顺序或临时的FileReader的使用寿命。有一些未知的,你应该看看:

  1. 是否的FileReader构造函数创建一个有效的缓冲,并将其存储在data
  2. getFileData()是否返回创建的缓冲区?
  3. GetDecompressedBufferSize()是否为有效缓冲区返回正确的值?
  4. 异常是否从WriteDecompressedBuffer引发,其代码我们没有?

最后,您可能想简化您的代码。这样的构造不太可读。而且,当然,使用像矢量这样的标准容器会使它更安全。

+0

感谢您抽出宝贵时间来完成此步骤。对于1和2是,对于3:该函数从缓冲区本身读出一个值。 4:发生异常是因为GetDecompressedBufferSize返回了一个错误的(巨大的)值。这是因为'CompressedBufferReader'得到了一个不正确的参数。我试图找出为什么正确的缓冲区没有发送到那里。 –

+0

我发现了罪魁祸首。这实际上是糟糕的指针算术。它确实发生在GetDecompressedBufferSize函数中。 –

3

您的代码违反了一个非常重要的规律,被称为“三巨头”

如果你的类有析构函数,赋值运算符的任何或复制 构造函数,那么它应该有所有三个其中

的原因是,如果一个自定义的析构函数存在,那么最有可能的自动合成赋值操作符和拷贝构造函数(即只是成员由成员拷贝构造或赋值)都不会是正确的事做。

这条规则非常重要,以至于如果你碰巧发现一个案例,例如一个析构函数,但是默认的拷贝构造函数是有意义的,那么你至少要在一个注释中写下你没有忘记复制构造函数和/或分配,但自动提供的将是正确的。

如果您的类不能被复制或复制构造,那么通过声明它是私有的并且不写入实现来禁止该操作。

在您的具体情况下,当您的两个类中的任何一个的实例被复制或分配时,指针将被复制,但数据将被销毁两次。

+1

再次阅读代码。 'reader'不是用'FileReader'构造的 - 完整的代码是:'reader(FileReader(filename).getFileData())',请注意'getFileData()'部分。当然,你错过了这个事实告诉我们关于代码的可读性的一些事情,但是这里没有做任何拷贝构造。 'reader'用'unsigned char *'初始化。 – eran

+0

这是一个耻辱...你可以标记最喜欢的问题,但不是最喜欢的答案。 – bitmask

+0

@eran:true。我删除了那部分。这段代码仍然很糟糕(例如,为什么使用所有那些显式管理不当的缓冲区而不是使用'std :: vector '?)。此外,通过这种代码,您如何确定在典型的UB演进过程中没有发生任何问题(即没有任何明显的情况发生,并且问题在以后只会在无辜的地方执行一百万条指令)? – 6502