wxWidgets / wxWidgets

Cross-Platform C++ GUI Library
https://www.wxwidgets.org/
5.75k stars 1.69k forks source link

update tif_fax3.[ch] to 3.5.7 #5668

Closed wxtrac closed 2 years ago

wxtrac commented 21 years ago

Issue migrated from trac ticket # 5668

priority: low

2002-12-01 21:34:18: jay_berkenbilt created the issue


I have some code that uses libtiff that has been working for some time on Linux. I have been porting this code to Windows for use in a wxWindows-based application. For the Windows port, I am using the tiff library as included within wxWindows.

In this version, I was seeing various memory errors (null pointer dereferences) only when reading CCITT group 4-compressed tiff files and only when reading them with TIFFReadScanline or TIFFReadEncodedStrip. (wxWindows uses TIFFReadRGBA* routines instead.)

I noticed that the tiff library code in wxWindows is based upon libtiff version 3.5.2. Since the current version of libtiff is 3.5.7, the first thing I tried was merging in changes between 3.5.2 and 3.5.7 into tiff_fax3.h and tiff_fax3.c. I was very careful to preserve changes such as those in #ifdef PURIFY, LINKAGEMODE, and VISAGECPP30. Building the tiff library and wxWindows with the resulting patched tif_fax3.[ch] corrects the problems I was seeing. My code's automated test suite exercises various parts of libtiff, and anyway, this is a very localized change, so the risk of breaking other code in tiff or wxwindows seems extremely slim. In addition to testing with my own test suite, I rebuilt the Image sample and used it to load some CCITT group 4 tiffs and other external tiff files. This works now, as it did prior to the patch.

As simply moving to the latest libtiff code solved my problem, I did not investigate its cause in depth. However, it is noteworthy that one of the changes to tif_fax3.c was to increase the amount of memory allocated in one instance.

My patch is relative to the tiff code in 2.2.9, but a quick check indicates that the current development branch is using the same tiff code.

wxtrac commented 21 years ago

2002-12-01 21:34:18: jay_berkenbilt uploaded file tiff-fax3-patch (6.0 KiB)

tif_fax3.[ch] patch to bring to 3.5.7

wxtrac commented 21 years ago

2002-12-06 22:37:26: @vadz commented


I don't really like the idea of having part of libtiff sources at 3.5.2 and part at 3.5.7 -- this risks to create a lot of confusion. Would it be possible for you to complete the patch to bring all the files to 3.5.7? I'd be very glad to apply it (to the cvs trunk only, however) then.

Thanks in advance!

wxtrac commented 21 years ago

2002-12-09 03:04:20: jay_berkenbilt commented


I don't blame you for being reluctant to have a hybrid of versions.

I would definitely consider bringing the tiff stuff up to 3.5.7.

Would there be any chance such a patch could make it into 2.4? If I can help make 2.4 a better release by working on updating the tiff code, I would make an effort to do this. How soon would you need it to make it useful? Of course, I'm not looking for a promise or guarantee. You have to feel free to choose whether to accept the patch on whatever basis you think is appropriate obviously.

I've just switched to 2.3.4 today from 2.2.9 for various reasons. The main reason is that I'm porting my automated test framework to work with wxWindows. wxApp.FilterEvent is essential as is the ability to use condition variables right (i.e,. with mutexes). If there were a reasonable chance of my patch making it into the next release, I'd be pretty motivated to do it. Otherwise, I may very well still do it, but it won't be as high a priority.

Thanks!

wxtrac commented 21 years ago

2002-12-09 03:11:08: @vadz commented


Unfortunately I don't think this patch can make it into 2.4.0 because the updates like this need to be tested under all platforms to ensure that they at least compile everywhere -- sorry!

To compensate for this I can only say that we will try to release 2.6.0 in less time than what has passed between 2.2.0 and 2.4.0...

Thanks for your understanding, VZ

wxtrac commented 21 years ago

2002-12-09 03:15:13: jay_berkenbilt commented


That's certainly reasonable. I'll still do the port if I can, but I'll be focusing on making sure my system is working properly with 2.3.4 first. Of course, when I do this, I'll send patches relative to the CVS trunk.

wxtrac commented 21 years ago

2002-12-09 18:05:44: jay_berkenbilt commented


Okay, I grabbed the CVS version of wxWindows and noticed that someone else has been trying to track down this problem and almost made the right fix. I have fixed the problem by changing one line of code. My patch changes the one line of code in question and removes the comment about not understanding the algorithm well enough to make the fix.

The one line of code is a change to the amount of memory allocated for nruns in tif_fax3.c. Someone already changed this enough to handle a specific case, but my fix, from 3.5.7, should be more accurate. Changing this single line of code is sufficient to get all my regression tests to pass.

This should be a much less intrusive fix. Hopefully this can make it into 2.4.0, but either way is fine ;-)

wxtrac commented 21 years ago

2002-12-09 18:07:18: jay_berkenbilt uploaded file tiff-fax3-from-trunk.patch (1.4 KiB)

wxtrac commented 21 years ago

2002-12-09 18:09:01: jay_berkenbilt commented


The second patch supercedes the first patch. Somehow the comment I entered for the patch got lost. I had said that the revised patch is relative to the trunk and supercedes the old patch.

wxtrac commented 21 years ago

2002-12-30 02:10:25: @vadz commented


I've applied the one line patch to both branches and will mark the bug as closed for now -- if you can make a patch upgrading the TIFF sources to 3.5.7, it would still be very appreciated. Please simply reopen the bug when/if you do it.

Thanks!