286233 - Cleanup nsImageWin::Init

Status: RESOLVED FIXED

(Core Graveyard :: GFX: Win32, defect)

Description

[ArronM (:paper)]

General cleanup.  Useless ifs, extra whitespace, lines hanging over 80 columns,
no null checking after memory allocation

Comment 1

[ArronM (:paper)]

Attachment: "diff -w" of cleanup

Asking for review on the 'ignore whitespace' patch, since it's way easier to
read (IMO).  Normal patch will also be attached to this bug.

Aside from formatting/space removal, this patch:
- Removes "if (mNumPaletteColors >= 0){".  mNumPaletteColors is never set
smaller  than 0.  there was code that set mNumPaletteColors to -1, but that has
since been removed sometime prior to April, 2000.

- an 'if' with two different memory allocations for mBHead has been changed to
use calculate a bHeadSize, and then allocate the memory based on that.	Looks
cleaner and removes the line that hangs over 80 columns.

- Exit with NS_ERROR_OUT_OF_MEMORY if memory allocation of mBHead or mImagebits
fails.


Also, I intentionally left out addressing the "// XXX Let's only do this if we
actually have a palette...".  I'll file a new bug for dealing with that
someday.

Comment 2

[ArronM (:paper)]

Attachment: normal diff of cleanup

Comment 3

[ArronM (:paper)]

Comment on attachment 177692 [details] [diff] [review]
"diff -w" of cleanup

murphy's law: You find an error in your patch within 5 minutes of posting it. 
Cancelling request.  New one coming soon

Comment 4

[ArronM (:paper)]

Attachment: "diff -w" of cleanup

Same as last patch, except I no longer touch the mBHead size calculation code
(except for formatting)

Comment 5

[ArronM (:paper)]

Attachment: normal diff of cleanup

Comment 6

[Ere Maijala (slow)]

Comment on attachment 177696 [details] [diff] [review]
"diff -w" of cleanup

Add check for success of
       mAlphaBits = new unsigned char[mARowBytes * aHeight];
too, and I'll probably be quite happy :)

Comment 7

[ArronM (:paper)]

Attachment: "diff -w" of cleanup

added check after mAlphaBits allocation

Comment 8

[ArronM (:paper)]

Attachment: normal diff of cleanup

Comment 9

[Ere Maijala (slow)]

Comment on attachment 178213 [details] [diff] [review]
"diff -w" of cleanup

Please make sure there's a space between |if (..)| and the left curly bracket.
With those fixed, r=emaijala

Comment 10

[ArronM (:paper)]

Attachment: normal diff of cleanup

update normal diff to put spaces after |if (..)| and {, as per ere's comment. 
-w patch doesn't change.

Comment 11

[tor]

Comment on attachment 178213 [details] [diff] [review]
"diff -w" of cleanup

mNumPaletteColors will always be 0 for now, but this is a good start to
cleanup.

Comment 12

[ArronM (:paper)]

Checking in gfx/src/windows/nsImageWin.cpp;
/cvsroot/mozilla/gfx/src/windows/nsImageWin.cpp,v  <--  nsImageWin.cpp
new revision: 3.143; previous revision: 3.142
done