Well, it's such horrible code it took me a while to find your problem:
read.read((char *)&Colors, (sizeof(byte) * channels));
Colors is already a pointer. You're not reading into the array Colors points at, your overwriting the Colors pointer itself. Give me a moment and I'll rewrite your code so that it works, is readable, is exception-safe and doesn't leak memory like nobody's business.
EDIT: It's not perfect by a long shot, but it's a lot better than your version. There are probably bugs - I didn't test it very thoroughly:
#include <string>#include <vector>// We're more interested in tga as an Image than as a File, so// name the class to reflect thatclass TgaImage{ public: // functions which create objects should usually be // constructors // std::string is better than char * because it allows // clients to use either a char * or a std::string // when creating a TgaImage, rather than only allowing // a char * TgaImage(std::string const & filename); // we don't want clients to arbitrarily change the // width, height or number of channels without actually // changing the data correctly, so make those values // read-only by encapsulating them and providing 'getters' unsigned short height() const; unsigned short number_of_channels() const; unsigned short width() const; private: // some helper functions void read_rle_tga_image(std::istream & reader); void read_16_bit_raw_tga_image(std::istream & reader); void read_24_or_32_bit_raw_tga_image(std::istream & reader); typedef unsigned char byte; unsigned short width_; unsigned short height_; unsigned short channels_; // vector is a dynamic array class. it's much easier // to use than raw pointers std::vector< byte > data_;};
#include "TgaImage.h"#include <cassert>#include <fstream>#include <stdexcept>// the unnamed namespace restricts the scope of variables and// functions to this translation unit (.cpp file) only.namespace{ typedef unsigned char byte; // prefer constants to #defines // constants respect scope, the preprocessor does not const byte type_rgb = 2; const byte type_alpha = 3; const byte type_rle = 10; // a simple templated function for binary file input template < typename Type > void read_binary(std::istream & reader, Type & value) { // prefer the C++ casts (static_cast, const_cast, // reinterpret_cast and dynamic_cast) to C-style casts // The C++ casts are more expressive, won't silently // do the wrong thing depending on context and are easy // to find reader.read(reinterpret_cast< char * >(&value), sizeof(Type)); // error checking if (reader.fail()) { throw std::runtime_error("error reading file"); } } // a specialization of the template for reading into a vector template < typename Type > void read_binary(std::istream & reader, std::vector< Type > & vector) { // don't want to index into an empty vector if (vector.empty()) { return; } reader.read(reinterpret_cast< char * >(&vector[0]), vector.size() * sizeof(Type)); if (reader.fail()) { throw std::runtime_error("error reading file"); } }}TgaImage::TgaImage(std::string const & filename){ // restrict using statements to the smallest scope possible using namespace std; // initialize at time of construction - it's often more // efficient and never less so. ifstream reader(filename.c_str(), ios::binary); // it is reasonable to expect loading a tga file to succeed // therefore an exception is a sensible approach if the // unexpected happens if (!reader) { throw std::runtime_error(("could not find tga file \'" + filename + "\'").c_str()); } // delay declaration of variables until they are needed // this helps keep the source more readable byte length; read_binary(reader, length); // i would prefer to use a symbolic constant here, but I don't // know tga files so I haven't a clue what that '1' is! reader.ignore(1); byte image_type; read_binary(reader, image_type); reader.ignore(9); // error checking - make sure that unsigned char really is // 16 bits. still not sufficient, since a byte on this // platform may not be eight bits, but it'll do assert(sizeof(width_) == 2 && sizeof(height_) == 2); read_binary(reader, width_); read_binary(reader, height_); byte bpp; read_binary(reader, bpp); reader.ignore(length + 1); if (bpp != 16 && bpp != 24 && bpp != 32) { throw std::runtime_error("could not load tga file - bits-per-pixel must be 16, 24 or 32"); } // since 16 bpp is a packed format with three channels we // have to correct this line. since the meaning is not // apparent from just reading the source a comment is // essential channels_ = std::max< byte >(bpp, 24) / 8; unsigned int stride = width_ * channels_; // allocate our image data array. vector is an RAII wrapper // it handles allocation and deallocation for us, so we can // just 'allocate and forget'. data_.resize(stride * height_); if (image_type == type_rle) { // can't handle 16 bpp rle files, so error out if (bpp == 16) { throw std::runtime_error("could not load tga file - rle image bits-per-pixel must be 24 or 32"); } // this is a logical place to break the function and stop // it getting too big and difficult to understand as a // whole read_rle_tga_image(reader); } else { if (bpp == 16) { read_16_bit_raw_tga_image(reader); } else { read_24_or_32_bit_raw_tga_image(reader); } }}unsigned short TgaImage::height() const{ return height_;}unsigned short TgaImage::number_of_channels() const{ return channels_;}unsigned short TgaImage::width() const{ return width_;}void TgaImage::read_rle_tga_image(std::istream & reader){ for (unsigned int i = 0; i < static_cast< unsigned int >(width_ * height_);) { byte rle_type; read_binary(reader, rle_type); if (rle_type < 128) { byte pixel_count = rle_type + 1; reader.read(reinterpret_cast< char * >(&data_), pixel_count * channels_);<br> <span class="cpp-keyword">if</span> (reader.fail())<br> {<br> <span class="cpp-keyword">throw</span> std::runtime_error(<span class="cpp-literal">"error reading file"</span>);<br> }<br> <span class="cpp-keyword">for</span> (<span class="cpp-keyword">unsigned</span> <span class="cpp-keyword">int</span> pixel = i * channels_; pixel < (i + pixel_count) * channels_; pixel += channels_)<br> {<br> std::swap(data_[pixel], data_[pixel + <span class="cpp-number">2</span>]);<br> }<br> i += pixel_count;<br> }<br> <span class="cpp-keyword">else</span><br> {<br> byte run_length = rle_type - <span class="cpp-number">127</span>;<br> byte colours[<span class="cpp-number">4</span>];<br> reader.read(<span class="cpp-keyword">reinterpret_cast</span>< <span class="cpp-keyword">char</span> * >(&colours), channels_);<br> <span class="cpp-keyword">if</span> (reader.fail())<br> {<br> <span class="cpp-keyword">throw</span> std::runtime_error(<span class="cpp-literal">"error reading file"</span>);<br> }<br> <span class="cpp-keyword">for</span> (byte pixel = <span class="cpp-number">0</span>; pixel < run_length; ++pixel)<br> {<br> data_[(i * channels_) + <span class="cpp-number">0</span>] = colours[<span class="cpp-number">2</span>];<br> data_[(i * channels_) + <span class="cpp-number">1</span>] = colours[<span class="cpp-number">1</span>];<br> data_[(i * channels_) + <span class="cpp-number">2</span>] = colours[<span class="cpp-number">0</span>];<br> <span class="cpp-keyword">if</span> (channels_ == <span class="cpp-number">4</span>)<br> {<br> data_[(i * channels_) + <span class="cpp-number">3</span>] = colours[<span class="cpp-number">3</span>];<br> }<br> i += channels_;<br> }<br> }<br> }<br>}<br><br><span class="cpp-keyword">void</span> TgaImage::read_16_bit_raw_tga_image(std::istream & reader)<br>{<br> <span class="cpp-keyword">for</span> (<span class="cpp-keyword">unsigned</span> <span class="cpp-keyword">int</span> i = <span class="cpp-number">0</span>; i < <span class="cpp-keyword">static_cast</span>< <span class="cpp-keyword">unsigned</span> <span class="cpp-keyword">int</span> >(width_ * height_); ++i)<br> {<br> <span class="cpp-keyword">unsigned</span> <span class="cpp-keyword">short</span> packed_pixel;<br> read_binary(reader, packed_pixel);<br><br> <span class="cpp-comment">// red component</span><br> data_[(i * channels_) + <span class="cpp-number">0</span>] = ((packed_pixel >> <span class="cpp-number">10</span>) & 0x1f) << <span class="cpp-number">3</span>;<br><br> <span class="cpp-comment">// green component</span><br> data_[(i * channels_) + <span class="cpp-number">1</span>] = ((packed_pixel >> <span class="cpp-number">5</span>) & 0x1f) << <span class="cpp-number">3</span>;<br><br> <span class="cpp-comment">// blue component</span><br> data_[(i * channels_) + <span class="cpp-number">2</span>] = ((packed_pixel >> <span class="cpp-number">0</span>) & 0x1f) << <span class="cpp-number">3</span>;<br> }<br>}<br><br><span class="cpp-keyword">void</span> TgaImage::read_24_or_32_bit_raw_tga_image(std::istream & reader)<br>{<br> read_binary(reader, data_);<br> <span class="cpp-keyword">for</span> (<span class="cpp-keyword">unsigned</span> <span class="cpp-keyword">int</span> i = <span class="cpp-number">0</span>; i < data_.size(); i += channels_)<br> {<br> std::swap(data_<span style="font-weight:bold;">, data_);<br> }<br>}<br></pre></div><!–ENDSCRIPT–><br><br>Σnigma<br><br><!–EDIT–><span class=editedby><!–/EDIT–>[Edited by - Enigma on June 12, 2006 5:22:37 PM]<!–EDIT–></span><!–/EDIT–>
