Bug 497492

Summary: mingw32-libjpeg crash on windows, typedef
Product: [Fedora] Fedora Reporter: Mikkel Kruse Johnsen <mikkel>
Component: mingw32-libjpegAssignee: Richard W.M. Jones <rjones>
Status: CLOSED RAWHIDE QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: low    
Version: rawhideCC: adam, berrange, erik-fedora, fedora-mingw, ilyes.gouta, lfarkas, rjones
Target Milestone: ---   
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2009-08-30 20:36:42 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Attachments:
Description Flags
Patch for SPEC file, to include jpeg-6b-typedefs.patch
none
Patch to fix typedef
none
Proposed patch to libjpeg-7 for solution 3
none
Patch to fix a typo which is in libjpeg-7
none
Patch against libtiff to use 'jpeg_boolean' instead of 'boolean'
none
Patch against gtk2 to use 'jpeg_boolean' instead of 'boolean' none

Description Mikkel Kruse Johnsen 2009-04-24 09:35:08 UTC
Created attachment 341131 [details]
Patch for SPEC file, to include jpeg-6b-typedefs.patch

Description of problem:
Testing with webkitgtk and loading pages containing jpeg images courses a crash

How reproducible:
Always

Steps to Reproduce:
1. Load ex http://arabic.cnn.com in webkitgtk
  
Actual results:
Crash

Expected results:
Should load

Additional info:

Comment 1 Mikkel Kruse Johnsen 2009-04-24 09:35:45 UTC
Created attachment 341132 [details]
Patch to fix typedef

Comment 2 Ilyes Gouta 2009-04-24 10:17:12 UTC
Hi,

Having boolean typedef'ed as an int is much more natural, at least for the C compiler. Can you post the faulty C code sequence which actually trigger the segfault? I'd like to see if we can fix the issue in an alternative way.

Regards,
Ilyes Gouta.

Comment 3 Erik van Pienbroek 2009-04-24 10:43:31 UTC
Removed the fedora-review flag as it should only be used for package reviews

Comment 4 Richard W.M. Jones 2009-04-24 14:09:23 UTC
I posted on the upstream libjpeg mailing list:

http://sourceforge.net/mailarchive/forum.php?forum_name=libjpeg-devel-6x

Comment 5 Kevin Kofler 2009-04-26 05:14:47 UTC
Looks like the problem here is that rpcndr.h and libjpeg both define a "boolean" type, and whoever is included first "wins". So programs including rpcndr.h before libjpeg will try using the wrong boolean type.

Comment 6 Bug Zapper 2009-06-09 14:30:38 UTC
This bug appears to have been reported against 'rawhide' during the Fedora 11 development cycle.
Changing version to '11'.

More information and reason for this action is here:
http://fedoraproject.org/wiki/BugZappers/HouseKeeping

Comment 7 Erik van Pienbroek 2009-08-22 15:21:55 UTC
Recently a new version of libjpeg was released (http://www.ijg.org/). Some code was added by upstream to circumvent the bug described here. However, this change was incomplete and contains a typo..so basically we're still stuck with the same problem.

Now there are multiple solutions to 'fix' this bug once and for all:

Solution 1:
-----------

Prevent the conflict by having jmorecfg.h contain 'typedef unsigned char boolean;'

+ No conflict anymore when using #include <rpcndr.h>
- The API of the libjpeg-7.dll will be different than what it is now
- All dependent packages need to be rebuild (libtiff, libjasper and gtk2)

Solution 2:
-----------

Don't use 'typedef int boolean;' when the file rpcndr.h is already included

+ No rebuild required for dependent packages (libtiff, libjasper and gtk2)
- This introduces side-effects when one source code file #include's rpcndr.h and another one doesn't

Solution 3:
-----------

Don't use a datatype named 'boolean', but something different like 'jpeg_boolean'

+ No conflicts anymore whether or not the file rpcndr.h is #include'd
+ Lower risk of side effects (the only risk which remains is the situation where a library or application #include's rpcndr.h and uses the 'boolean' datatype, but for those situations the compiler should give a warning anyway)
+ The API of the libjpeg-7.dll remains the same (the size of all the data structures remain the same)
- May require patching of libraries/applications which use libjpeg


My vote goes to the third solution. That solution is also one which 'could' be proposed upstream (be it for a future major version of libjpeg as it may require changes in libraries or applications using libjpeg).

Patches are attached

Comment 8 Erik van Pienbroek 2009-08-22 15:23:02 UTC
Created attachment 358307 [details]
Proposed patch to libjpeg-7 for solution 3

Comment 9 Erik van Pienbroek 2009-08-22 15:32:43 UTC
Created attachment 358308 [details]
Patch to fix a typo which is in libjpeg-7

There also was a conflict between libjpeg and basestd.h regarding the datatype 'INT32'. In libjpeg-7 some code was added to fix this problem, but this fix contains a typo.

Attentive readers might have noticed that basetsd.h contains a 'typedef int INT32' while libjpeg uses a 'typedef long INT32'. For Win32 this doesn't matter as sizeof(int) == sizeof(long) on those environments.

Comment 10 Erik van Pienbroek 2009-08-22 15:33:39 UTC
Created attachment 358309 [details]
Patch against libtiff to use 'jpeg_boolean' instead of 'boolean'

Comment 11 Erik van Pienbroek 2009-08-22 15:34:38 UTC
Created attachment 358310 [details]
Patch against gtk2 to use 'jpeg_boolean' instead of 'boolean'

Comment 12 Richard W.M. Jones 2009-08-22 16:06:39 UTC
Do we know what the situation is with upstream jpeg?  Where did jpeg 7
come from?  The mailing list mentioned in comment 4 still looks pretty
dead.

My point is that if upstream has somehow come alive enough to do
a release, then we should ask them to fix the typo (or what the best
fix is).

Comment 13 Erik van Pienbroek 2009-08-22 17:05:00 UTC
(In reply to comment #12)
> Do we know what the situation is with upstream jpeg?  Where did jpeg 7
> come from?  The mailing list mentioned in comment 4 still looks pretty
> dead.
> 
> My point is that if upstream has somehow come alive enough to do
> a release, then we should ask them to fix the typo (or what the best
> fix is).  

To be honest, I absolutely haven't got a clue... The ChangeLog only mentions the IJG group. The sourceforge page only refers to version 6. The code which is in the sourceforge CVS is a different codebase than the libjpeg-7 release from the IJG website...

Comment 14 Erik van Pienbroek 2009-08-23 12:34:59 UTC
If there are no objections I'm going to apply these patches tomorrow evening (Monday August 24) to Rawhide

Comment 15 Erik van Pienbroek 2009-08-30 20:36:42 UTC
The patches which were proposed here have been applied to rawhide at Thursday August 27 in mingw32-libjpeg-7-1.fc12

Comment 16 Adam Goode 2010-06-03 19:27:42 UTC
This is a pretty big problem, this does change the API of libjpeg such that I cannot build my code both natively on Fedora and with mingw32. I am using my own src manager, so the function I define has to either return boolean (on all libjpeg except this one) or jpeg_boolean (on the Fedora patched mingw32-libjpeg).

Comment 17 Adam Goode 2010-06-03 19:50:23 UTC
Also, why is this necessary just for the mingw32 package? The native package has no such patch.