Closed Bug 286233 Opened 20 years ago Closed 19 years ago

Cleanup nsImageWin::Init

Categories

(Core Graveyard :: GFX: Win32, defect)

x86
Windows 2000
defect
Not set
minor

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: paper, Assigned: paper)

Details

Attachments

(2 files, 5 obsolete files)

General cleanup.  Useless ifs, extra whitespace, lines hanging over 80 columns,
no null checking after memory allocation
Attached patch "diff -w" of cleanup (obsolete) — Splinter Review
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.
Attachment #177692 - Flags: review?(emaijala)
Attached patch normal diff of cleanup (obsolete) — Splinter Review
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
Attachment #177692 - Flags: review?(emaijala)
Attached patch "diff -w" of cleanup (obsolete) — Splinter Review
Same as last patch, except I no longer touch the mBHead size calculation code
(except for formatting)
Attachment #177692 - Attachment is obsolete: true
Attachment #177696 - Flags: review?(emaijala)
Attached patch normal diff of cleanup (obsolete) — Splinter Review
Attachment #177693 - Attachment is obsolete: true
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 :)
Attachment #177696 - Flags: review?(emaijala) → review-
added check after mAlphaBits allocation
Attachment #177696 - Attachment is obsolete: true
Attachment #178213 - Flags: review?(emaijala)
Attached patch normal diff of cleanup (obsolete) — Splinter Review
Attachment #177697 - Attachment is obsolete: true
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
Attachment #178213 - Flags: review?(emaijala) → review+
update normal diff to put spaces after |if (..)| and {, as per ere's comment. 
-w patch doesn't change.
Attachment #178214 - Attachment is obsolete: true
Attachment #178213 - Flags: superreview?(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.
Attachment #178213 - Flags: superreview?(tor) → superreview+
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
Status: NEW → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: