Closed Bug 1072340 Opened 6 years ago Closed 6 years ago

Clean up PNG codecs, use PR_LOG not PR_fprintf for errors

Categories

(Core :: ImageLib, defect)

defect
Not set
minor

Tracking

()

VERIFIED FIXED
mozilla35

People

(Reporter: glennrp+bmo, Assigned: glennrp+bmo)

References

Details

Attachments

(1 file, 4 obsolete files)

The PNG codecs could use some minor cleanup:

 The encoder uses PR_fprintf for error messages while the decoder uses PR_LOG.
 longjmp() should be replaced with png_longjmp().
 Support for libpng versions prior to 1.5.0 can be removed.
 There is an #if 0/#endif block that can be removed.
 There are a couple of trailing blanks, that should be removed.
 There are some long lines that need to be wrapped.
Attached patch v00 clean up image/png codecs (obsolete) — Splinter Review
Attached patch v01 clean up image/png codecs (obsolete) — Splinter Review
I uploaded the wrong file; the v00 patch was truncated.
Attachment #8494557 - Attachment is obsolete: true
Depends on: 1021713
No longer depends on: 1063229
Attached patch v02 clean up image/png codecs (obsolete) — Splinter Review
Wrapped a couple of overlooked long lines; alphabetized "#include" lines, changed some remaining c-style comments to "//"
Attachment #8494560 - Attachment is obsolete: true
Attached patch v02 clean up image/png codecs (obsolete) — Splinter Review
Attachment #8494616 - Attachment is obsolete: true
Attachment #8494616 - Attachment description: v01 clean up image/png codecs → v02 clean up image/png codecs
Adds header guard nsPNGEncoder_h
Changes header guard nsPNGDecoder_h__ to nsPNGDecoder_h
Alphabetizes #include directives where possible
Attachment #8494617 - Attachment is obsolete: true
I don't have plans for any further updates to this patch.  Time for a try-server run?  It builds fine on my local 64-bit Ubuntu 14.04LTS platform.
Flags: needinfo?(ryanvm)
(In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #7)
> https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=239efc2fde8a

Returns "Unknown revision ID."  Does that mean there's something wrong
with the patch, or something wrong with treeherder?
Flags: needinfo?(ryanvm)
We were having infra issues yesterday that caused TH to miss pushes.

https://tbpl.mozilla.org/?tree=Try&rev=239efc2fde8a
Flags: needinfo?(ryanvm)
Try was green.
Attachment #8494889 - Flags: review?(jmuizelaar)
Attachment #8494889 - Flags: review?(jmuizelaar) → review+
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/31d28b1d4d7e
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla35
Blocks: 991149
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.