645519 - Firefox-4.0 compile fails if "--with-system-png" is ON

Status: RESOLVED FIXED

(Core :: Graphics: ImageLib, defect)

Description

[Alex]

User-Agent:       Opera/9.80 (X11; Linux i686; U; en) Presto/2.7.62 Version/11.01
Build Identifier: Mozilla/5.0 (X11; Linux i686; rv:2.0) Gecko/20100101 Firefox/4.0

Xorg-7.6
glib-2.28.4
gtk+-2.24.3
sqlite-3.7.5
nspr-4.8.7
nss-3.12.9
libpng-1.5.1 +apng
cairo-1.10.2 +tee
---------------------------------------------------------
.mozconfig
mk_add_options MOZ_OBJDIR=@TOPSRCDIR@/../firefox-build
mk_add_options MOZ_MAKE_FLAGS="-j4"
ac_add_options --prefix=/usr
ac_add_options --disable-accessibility
ac_add_options --disable-auto-deps
ac_add_options --disable-crashreporter
ac_add_options --disable-dbm
ac_add_options --disable-gnomevfs
ac_add_options --disable-installer
ac_add_options --disable-tests
ac_add_options --disable-updater
ac_add_options --enable-cpp-rtti
ac_add_options --enable-default-toolkit=cairo-gtk2
ac_add_options --enable-image-decoders=all
ac_add_options --enable-image-encoders=all
ac_add_options --enable-jemalloc
ac_add_options --enable-official-branding
ac_add_options --enable-optimize
ac_add_options --enable-places
ac_add_options --enable-safe-browsing
ac_add_options --enable-strip
ac_add_options --enable-system-cairo
ac_add_options --enable-system-lcms
ac_add_options --enable-system-sqlite
ac_add_options --enable-valgrind
ac_add_options --enable-libxul
ac_add_options --with-java-bin-path=/opt/jdk/bin
ac_add_options --with-java-include-path=/opt/jdk/include
ac_add_options --with-qtdir=/opt/qt
ac_add_options --with-system-bz2
ac_add_options --with-system-jpeg
ac_add_options --with-system-nspr
ac_add_options --with-system-nss
ac_add_options --with-system-png
ac_add_options --with-system-zlib
ac_add_options --with-x
===============================================================================
ERROR
In '/usr/src/firefox-build/modules/libpr0n/decoders'

c++ -o nsPNGDecoder.o -c  -fvisibility=hidden -DMOZ_PNG_WRITE -DMOZ_PNG_READ  -DMOZILLA_INTERNAL_API -D_IMPL_NS_COM -DEXPORT_XPT_API -DEXPORT_XPTC_API -D_IMPL_NS_GFX -D_IMPL_NS_WIDGET -DIMPL_XREAPI -DIMPL_NS_NET -DIMPL_THEBES  -DSTATIC_EXPORTABLE_JS_API -DOSTYPE=\"Linux2.6.37\" -DOSARCH=Linux -I/usr/src/mozilla-2.0/modules/libpr0n/src/ -I/usr/src/mozilla-2.0/modules/libpr0n/decoders -I. -I../../../dist/include -I../../../dist/include/nsprpub  -I/usr/include/nspr -I/usr/include/nss       -fPIC  -frtti -fno-exceptions -Wall -Wpointer-arith -Woverloaded-virtual -Wsynth -Wno-ctor-dtor-privacy -Wno-non-virtual-dtor -Wcast-align -Wno-invalid-offsetof -Wno-variadic-macros -pedantic -Wno-long-long -fno-strict-aliasing -fshort-wchar -pthread -pipe  -DNDEBUG -DTRIMMED -Os -freorder-blocks -fomit-frame-pointer -finline-limit=50   -DMOZILLA_CLIENT -include ../../../mozilla-config.h /usr/src/mozilla-2.0/modules/libpr0n/decoders/nsPNGDecoder.cpp

/usr/src/mozilla-2.0/modules/libpr0n/decoders/nsPNGDecoder.cpp: In function 'qcms_profile* mozilla::imagelib::PNGGetColorProfile(png_struct*, png_info*, int, qcms_data_type*, PRUint32*)':
/usr/src/mozilla-2.0/modules/libpr0n/decoders/nsPNGDecoder.cpp:393: error: invalid conversion from 'char**' to 'png_byte**'

make[5]: *** [nsPNGDecoder.o] Error 1
make[5]: Leaving directory `/usr/src/firefox-build/modules/libpr0n/decoders'
 ...
-------------------------------------------------------------------------------
PROBLEM (as seen by me)
Excerpt from 'mozilla-2.0/modules/libpr0n/decoders/nsPNGDecoder.cpp'
  ...
Line #386
  // First try to see if iCCP chunk is present
  if (png_get_valid(png_ptr, info_ptr, PNG_INFO_iCCP)) {
    png_uint_32 profileLen;
    char *profileData, *profileName;
    int compression;

    png_get_iCCP(png_ptr, info_ptr, &profileName, &compression,
                 &profileData, &profileLen);

    profile = qcms_profile_from_memory(profileData, profileLen);
    ...
-------------------------------------------------------------------------
FIX (workaround?)
Changed "profileData" from "pointer to char" to "pointer to png_byte":
    ...
    char *profileName;
    png_byte *profileData;
    int compression;
    ...

Once I applied my "fix",
 the compile ('make -f client.mk build') quieted down completely.

BTW, png_byte is defined all over the place (and libpng versions) as
(say, in '/usr/include/pngconf.h')
typedef unsigned char png_byte;
so my fix appears relatively safe.  But who knows?

COMMENT
This "bug" is in a function commented by the author as
 "Adapted from http://www.littlecms.com/pngchrm.c example code"

I don't know if that's a good sign.

-- Alex

Reproducible: Always

Steps to Reproduce:
1.  make -f client.mk build
2.
3.
Actual Results:  
See "Details" above

Expected Results:  
For the compile (make build) to work.

Comment 1

[Mats Palmgren (inactive)]

It appears the parameter type was changed recently from png_charpp
to png_bytepp:
http://libpng.git.sourceforge.net/git/gitweb.cgi?p=libpng/libpng;a=commitdiff;h=9d23b40c24347c2ca04a2a4ff9c4d2f13855f2c0

Your suggested fix would break builds with the older versions of libpng.
Not sure how to fix this without resorting to #if PNG_LIBPNG_VER... 

Glenn?

Comment 2

[Alex]

For reference only (to eliminate any possible confusion):
'firefox-4.0.source.tar.bz2'
MD5sum: 3468a2c463b4fc2788ba621e4b511c30

Hi Mats,

My investigation and the consequent primitive "FIX" was triggered by
a debate where one user (Andrew Benton - from (B)LFS) had no problems
compiling with 1.4.4 while I was being harshly punished for using the
up-to-date 1.5.1
(let alone the reigning confusion in 'configure --help' where
--with-system-png appears to function differently than --enable-system-png)
Be that as it may,
(I know you were talking mostly to Glenn but) AFAIAK I'd be fine with
#if PNG_LIBPNG_VER...

As a small rant, the 'configure' parse should always have something like
(in this particular case)
"Error - Need libpng >= 1.4.0 and <1.5.0"
"We don't have no use for eager-beavers in these here parts"

It happens with Firefox and GTK (to cite two main offenders that come to mind) way too many times (the version problems as above).

Anyway, thank you for your words.
-- Alex

Comment 3

[Glenn Randers-Pehrson]

In other applications (e.g., ImageMagick) I used
#if (PNG_LIBPNG_VER < 10500)
So I suppose we need to do that here as well.

Comment 4

[Glenn Randers-Pehrson]

Attachment: Declare "pngbytep profileData" when libpng is 1.5 or later

Comment 5

[Glenn Randers-Pehrson]

Comment on attachment 522556 [details] [diff] [review]
Declare "pngbytep profileData" when libpng is 1.5 or later

Oops, I have to redo this (profileName is still char*)

Comment 6

[Glenn Randers-Pehrson]

Attachment: Declare "pngbytep profileData" when libpng is 1.5 or later (v01)

Comment 7

[Joe Drew (not getting mail)]

Comment on attachment 522560 [details] [diff] [review]
Declare "pngbytep profileData" when libpng is 1.5 or later (v01)

If you wanted, you could unconditionally cast to (char *), since it's a no-op if profileData's already a char *. That'd probably have to be paired with a comment too, though, so it's 6 of one, half-dozen of the other.

Comment 8

[Glenn Randers-Pehrson]

Yes, either would work.  Let's just leave it as in the v01 patch.

Comment 9

[(no longer active)]

http://hg.mozilla.org/projects/cedar/rev/a60e842a1561

Comment 10

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

http://hg.mozilla.org/mozilla-central/rev/a60e842a1561