Clean up PNG codecs, use PR_LOG not PR_fprintf for errors

VERIFIED FIXED in mozilla35

Status

()

defect
--
minor
VERIFIED FIXED
5 years ago
5 years ago

People

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

Tracking

Trunk
mozilla35
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 4 obsolete attachments)

Assignee

Description

5 years ago
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.
Assignee

Comment 1

5 years ago
Posted patch v00 clean up image/png codecs (obsolete) — Splinter Review
Assignee

Comment 2

5 years ago
Posted 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
Assignee

Updated

5 years ago
Depends on: 1021713
No longer depends on: 1063229
Assignee

Comment 3

5 years ago
Posted 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
Assignee

Comment 4

5 years ago
Posted patch v02 clean up image/png codecs (obsolete) — Splinter Review
Attachment #8494616 - Attachment is obsolete: true
Assignee

Updated

5 years ago
Attachment #8494616 - Attachment description: v01 clean up image/png codecs → v02 clean up image/png codecs
Assignee

Comment 5

5 years ago
Adds header guard nsPNGEncoder_h
Changes header guard nsPNGDecoder_h__ to nsPNGDecoder_h
Alphabetizes #include directives where possible
Attachment #8494617 - Attachment is obsolete: true
Assignee

Comment 6

5 years ago
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)
Assignee

Comment 8

5 years ago
(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)
Assignee

Comment 10

5 years ago
Try was green.
Assignee

Updated

5 years ago
Attachment #8494889 - Flags: review?(jmuizelaar)
Attachment #8494889 - Flags: review?(jmuizelaar) → review+
Assignee

Updated

5 years ago
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/31d28b1d4d7e
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla35
Assignee

Updated

5 years ago
Blocks: 991149
Assignee

Updated

5 years ago
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.