• Create Account

### #ActualCornstalks

Posted 31 December 2012 - 05:46 PM

Well, for one, you're never setting pixelData. This is the same problem that I said bitmap had. The thing about pointers is, if you don't give them a chunk of valid memory to point to, they're just pointing off into oblivion, and using them is a great way to crash your program. You need to allocate memory for pixelData if you're going to write to it.

Also, this:

while(padWidth % 4 != 0)
{
}


can be simplified to:

padWidth += padWidth % 4; // round up to multiple of 4


Actually, the more I look at it there's a whole bunch of problems with your code. colors and tempPixelData leak memory because you never delete [] what you new [] (there could be other leaks elsewhere; I haven't checked too thoroughly). You're mixing C with C++ when you shouldn't (for example, don't use FILEs, use std::fstreams, and use  std::copy instead of memcpy). But even if you keep using FILEs, you're not fclose()ing what you fopen().

And this if statement is useless:

if(tempPixelData == NULL)
{
fclose(in);
return false;
}

If new fails, it will throw an exception (std::bad_alloc), so there's no point in checking it for NULL.

Also, you should place your #includes inside of your inclusion guards (this isn't necessarily a problem, but it's "best practice").

### #1Cornstalks

Posted 31 December 2012 - 05:44 PM

Well, for one, you're never setting pixelData. This is the same problem that I said bitmap had. The thing about pointers is, if you don't give them a chunk of valid memory to point to, they're just pointing off into oblivion, and using them is a great way to crash your program. You need to allocate memory for pixelData if you're going to write to it.

Also, this:

while(padWidth % 4 != 0)
{
}


can be simplified to:

padWidth += padWidth % 4; // round up to multiple of 4


Actually, the more I look at it there's a whole bunch of problems with your code. colors and tempPixelData leak memory because you never delete [] what you new [] (there could be other leaks elsewhere; I haven't checked too thoroughly). You're mixing C with C++ when you shouldn't; don't use FILEs, use std::fstreams, memcpy instead of std::copy, etc. But even if you keep using FILEs, you're not fclose()ing what you fopen().

This if statement is useless:

if(tempPixelData == NULL)</div>
<div>{</div>
<div>    fclose(in);</div>
<div>    return false;</div>
<div>}

If new fails, it will throw an exception (std::bad_alloc), so there's no point in checking it for NULL.

Also, you should place your #includes inside of your inclusion guards (this isn't necessarily a problem, but it's "best practice").

PARTNERS