Clean up PNG codecs, use PR_LOG not PR_fprintf for errors

VERIFIED FIXED in mozilla35

Status

()

Core
ImageLib
--
minor
VERIFIED FIXED
3 years ago
3 years ago

People

(Reporter: Glenn Randers-Pehrson, Assigned: Glenn Randers-Pehrson)

Tracking

Trunk
mozilla35
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 4 obsolete attachments)

(Assignee)

Description

3 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

3 years ago
Created attachment 8494557 [details] [diff] [review]
v00 clean up image/png codecs
(Assignee)

Comment 2

3 years ago
Created attachment 8494560 [details] [diff] [review]
v01 clean up image/png codecs

I uploaded the wrong file; the v00 patch was truncated.
Attachment #8494557 - Attachment is obsolete: true
(Assignee)

Updated

3 years ago
Depends on: 1021713
No longer depends on: 1063229
(Assignee)

Comment 3

3 years ago
Created attachment 8494616 [details] [diff] [review]
v02 clean up image/png codecs

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

3 years ago
Created attachment 8494617 [details] [diff] [review]
v02 clean up image/png codecs
Attachment #8494616 - Attachment is obsolete: true
(Assignee)

Updated

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

Comment 5

3 years ago
Created attachment 8494889 [details] [diff] [review]
v03 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
(Assignee)

Comment 6

3 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

3 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

3 years ago
Try was green.
(Assignee)

Updated

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

Updated

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

Updated

3 years ago
Blocks: 991149
(Assignee)

Updated

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