Jump to content

  • Log In with Google      Sign In   
  • 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)
{
    padWidth++;
}

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)
{
    padWidth++;
}

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