Closed Bug 623796 Opened 9 years ago Closed 9 years ago

nsPNGDecoder.cpp:642:44: warning: passing NULL to non-pointer argument 2 of 'void MOZ_PNG_set_crc_action(png_struct*, int, int)'

Categories

(Core :: ImageLib, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla2.0b11

People

(Reporter: dholbert, Assigned: glennrp+bmo)

References

Details

(Whiteboard: [build_warning])

Attachments

(1 file)

When building mozilla-central, I get this build warning:

> modules/libpr0n/decoders/nsPNGDecoder.cpp: In static member function 'static void mozilla::imagelib::nsPNGDecoder::info_callback(png_struct*, png_info*)':
> modules/libpr0n/decoders/nsPNGDecoder.cpp:642:44: warning: passing NULL to non-pointer argument 2 of 'void MOZ_PNG_set_crc_action(png_struct*, int, int)'

The chunk of code involved is:
===
>637   /* Reject any ancillary chunk after IDAT with a bad CRC (bug #397593).
>638    * It would be better to show the default frame (if one has already been
>639    * successfully decoded) before bailing, but it's simpler to just bail
>640    * out with an error message.
>641    */
>642   png_set_crc_action(png_ptr, NULL, PNG_CRC_ERROR_QUIT);
===
and it was added in bug 397593, as noted in the comment.

Presumably this was just a typo & we should be passing 0 instead of NULL as the second argument there...?
(or did the called function used to take a pointer whereas now it takes an integer?)
FWIW, this appears to be the only call to png_set_crc_action in all of mozilla-central, based on this search:
http://mxr.mozilla.org/mozilla-central/search?string=png_set_crc_action
(In reply to comment #0)
> Presumably this was just a typo & we should be passing 0 instead of NULL as the
> second argument there...?

or rather, presumably we should be passing PNG_CRC_DEFAULT, the PNG flag that corresponds to a "0"/NULL value in png.h...?

That is, assuming we want the crit_action==PNG_CRC_DEFAULT behavior in png_set_crc_action (which is the behavior that NULL is giving us right now):
>34    switch (crit_action)
...
>55       case PNG_CRC_DEFAULT:
>56       default:
>57          png_ptr->flags &= ~PNG_FLAG_CRC_CRITICAL_MASK;
>58          break;
crit_action is an int.  The one we really want is PNG_CRC_NO_CHANGE, which
happens to be 5 (defined in png.h).

so: png_set_crc_action(png_ptr, PNG_CRC_NO_CHANGE, PNG_CRC_ERROR_QUIT);

since we have never changed the action from default, this would just leave it so,
without resetting the flag.
Status: NEW → ASSIGNED
(I'm assuming that your NEW --> ASSIGNED bug-status change indicated that you meant to assign it to yourself, so I'm adding you as the assignee.  Feel free to revert if that's not correct.)
Assignee: nobody → glennrp+bmo
Yes that's what I intended.  I'll prepare the (trivial) patch shortly.
Attachment #501918 - Flags: review?(joe)
OS: Linux → All
Hardware: x86_64 → All
Attachment #501918 - Flags: review?(joe) → review+
Keywords: checkin-needed
This needs approval before it can land.
Keywords: checkin-needed
Attachment #501918 - Flags: approval2.0?
Attachment #501918 - Flags: approval2.0? → approval2.0+
Keywords: checkin-needed
http://hg.mozilla.org/mozilla-central/rev/27e81aa25025
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0b11
You need to log in before you can comment on or make changes to this bug.