<AWT Dev> RFR: 8077982: giflib upgrade breaks system giflib builds with earlier versions

[Phil Race]

I think you meant the subject to be "RFR: 8081315 ..."

if (DGifCloseFile(gif, NULL) == GIF_ERROR)
           return 0;

I suppose we probably ought to be checking the (new) error returns
although I am not sure what impact a failure on this particular
call would have if we already had read the data.

But could you write this as

if (DGifCloseFile(gif, NULL) == GIF_ERROR){
     return 0;
}

?


-phil.


On 06/02/2015 09:26 AM, Andrew Hughes wrote:
> Bug: https://bugs.openjdk.java.net/browse/JDK-8081315
> Webrev: http://cr.openjdk.java.net/~andrew/8081315/webrev.01/
>
> In 8011278, the option was added to build OpenJDK with a system
> installation of giflib, something we've been doing for many years
> in the IcedTea project.
>
> Following 8077982, the internal copy of giflib was upgraded to
> version 5.1 and appropriate changes were made to the splashscreen
> code to adapt to this version. However, these changes were made
> unconditionally and, in the process, the build of OpenJDK with
> a system version of giflib < 5 was broken.
>
> This is because giflib 5.0 altered the signature of DGifOpen to
> take an optional error code and the same change was made to
> DGifCloseFile in 5.1. Such changes were made upstream to allow
> the use of giflib from multi-threaded applications where a
> shared error number may cause concurrency issues.
>
> We adapted IcedTea to these changes in 2.3.7 (2013-02-20) and
> 2.5.5 (2015-04-14) using conditionals to allow the code to still
> build on older versions. A slightly updated version of this
> code forms the webrev included here. As can be seen from the patch,
> the same code is largely retained for the in-tree giflib 5, with
> the minor addition of checking for an error from DGifCloseFile.
> However, the conditionals also allow the return of the older
> code used prior to 8077982, so that older system giflibs can
> still be used.
>
> Does this look OK to push to client?
>
> Thanks,