From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: Eli Zaretskii Newsgroups: gmane.emacs.bugs Subject: bug#17790: GIFlib-5.1.0 or former conditional Date: Tue, 17 Jun 2014 17:59:18 +0300 Message-ID: <83a99bvayh.fsf@gnu.org> References: <0l38f56c2h.fsf@fencepost.gnu.org> Reply-To: Eli Zaretskii NNTP-Posting-Host: plane.gmane.org X-Trace: ger.gmane.org 1403017478 27178 80.91.229.3 (17 Jun 2014 15:04:38 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Tue, 17 Jun 2014 15:04:38 +0000 (UTC) Cc: 17790@debbugs.gnu.org To: Andreas Schwab , "Eric S. Raymond" Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Tue Jun 17 17:04:30 2014 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane.org Original-Received: from lists.gnu.org ([208.118.235.17]) by plane.gmane.org with esmtp (Exim 4.69) (envelope-from ) id 1Wwuw2-0006IX-2H for geb-bug-gnu-emacs@m.gmane.org; Tue, 17 Jun 2014 17:04:30 +0200 Original-Received: from localhost ([::1]:51219 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Wwuw1-0008AR-Lr for geb-bug-gnu-emacs@m.gmane.org; Tue, 17 Jun 2014 11:04:29 -0400 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:44709) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Wwurr-0001zP-5y for bug-gnu-emacs@gnu.org; Tue, 17 Jun 2014 11:00:16 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Wwurl-0005Rm-II for bug-gnu-emacs@gnu.org; Tue, 17 Jun 2014 11:00:11 -0400 Original-Received: from debbugs.gnu.org ([140.186.70.43]:59598) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Wwurl-0005Pb-FX for bug-gnu-emacs@gnu.org; Tue, 17 Jun 2014 11:00:05 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.80) (envelope-from ) id 1Wwurk-0004A9-2I for bug-gnu-emacs@gnu.org; Tue, 17 Jun 2014 11:00:04 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Eli Zaretskii Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Tue, 17 Jun 2014 15:00:03 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 17790 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: Original-Received: via spool by 17790-submit@debbugs.gnu.org id=B17790.140301717615927 (code B ref 17790); Tue, 17 Jun 2014 15:00:03 +0000 Original-Received: (at 17790) by debbugs.gnu.org; 17 Jun 2014 14:59:36 +0000 Original-Received: from localhost ([127.0.0.1]:50748 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.80) (envelope-from ) id 1WwurD-00048l-Op for submit@debbugs.gnu.org; Tue, 17 Jun 2014 10:59:35 -0400 Original-Received: from mtaout24.012.net.il ([80.179.55.180]:32943) by debbugs.gnu.org with esmtp (Exim 4.80) (envelope-from ) id 1Wwur8-00048T-6P for 17790@debbugs.gnu.org; Tue, 17 Jun 2014 10:59:30 -0400 Original-Received: from conversion-daemon.mtaout24.012.net.il by mtaout24.012.net.il (HyperSendmail v2007.08) id <0N7B00600IKIZR00@mtaout24.012.net.il> for 17790@debbugs.gnu.org; Tue, 17 Jun 2014 17:55:15 +0300 (IDT) Original-Received: from HOME-C4E4A596F7 ([87.69.4.28]) by mtaout24.012.net.il (HyperSendmail v2007.08) with ESMTPA id <0N7B00ORMIS2ZU70@mtaout24.012.net.il>; Tue, 17 Jun 2014 17:55:15 +0300 (IDT) In-reply-to: X-012-Sender: halo1@inter.net.il X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.15 Precedence: list X-detected-operating-system: by eggs.gnu.org: GNU/Linux 3.x X-Received-From: 140.186.70.43 X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Original-Sender: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Xref: news.gmane.org gmane.emacs.bugs:90476 Archived-At: > From: Andreas Schwab > Date: Tue, 17 Jun 2014 09:05:00 +0200 > Cc: 17790@debbugs.gnu.org > > Makoto Fujiwara writes: > > > +#if (GIFLIB_MAJOR == 5) && (GIFLIB_MINOR > 0) > > What about libgif6? Please remove the extra parens. > > > + int * return_value_p; > > This is not a predicate, please remove _p. > > > +#endif > > /* Before reading entire contents, check the declared image size. */ > > if (!check_image_size (f, gif->SWidth, gif->SHeight)) > > { > > image_error ("Invalid image size (see `max-image-size')", Qnil, Qnil); > > +#if (GIFLIB_MAJOR == 5) && (GIFLIB_MINOR > 0) > > + fn_DGifCloseFile (gif, return_value_p); > > + return *return_value_p; > > What is the meaning of the return value? Is it compatible with the > intented bool value? No, it isn't. Also, no other image loading method returns values specific to some image type to its caller, so I don't think it's a good idea. Moreover, in most of the places where we call DGifCloseFile, we don't really care about any errors it encounters, because we are already in the context of a more significant error. So it is better to simply disregard the errors there. I propose a slightly different changeset below. Unless someone sees some problem with it, I will commit soon. (I guess this should go to the release branch, right? Stefan? Glenn?) Finally, a question to Eric: can we rely on DGifCloseFile to accept NULL as the second argument, like the other APIs do? This is not documented, but the source clearly says that the behavior is the same as with the other APIs. Here's the suggested patch: === modified file 'src/image.c' --- src/image.c 2014-05-04 18:51:32 +0000 +++ src/image.c 2014-06-17 14:50:54 +0000 @@ -7255,7 +7255,11 @@ gif_image_p (Lisp_Object object) #ifdef WINDOWSNT /* GIF library details. */ +#if 5 < GIFLIB_MAJOR + (1 <= GIFLIB_MINOR) +DEF_IMGLIB_FN (int, DGifCloseFile, (GifFileType *, int *)); +#else DEF_IMGLIB_FN (int, DGifCloseFile, (GifFileType *)); +#endif DEF_IMGLIB_FN (int, DGifSlurp, (GifFileType *)); #if GIFLIB_MAJOR < 5 DEF_IMGLIB_FN (GifFileType *, DGifOpen, (void *, InputFunc)); @@ -7325,6 +7329,19 @@ gif_read_from_memory (GifFileType *file, return len; } +static int +gif_close (GifFileType *gif, int *err) +{ + int retval; + +#if 5 < GIFLIB_MAJOR + (1 <= GIFLIB_MINOR) + retval = fn_DGifCloseFile (gif, err); +#else + retval = fn_DGifCloseFile (gif); + if (err) + *err = gif->Error; +#endif +} /* Load GIF image IMG for use on frame F. Value is true if successful. */ @@ -7419,7 +7436,7 @@ gif_load (struct frame *f, struct image if (!check_image_size (f, gif->SWidth, gif->SHeight)) { image_error ("Invalid image size (see `max-image-size')", Qnil, Qnil); - fn_DGifCloseFile (gif); + gif_close (gif, NULL); return 0; } @@ -7428,7 +7445,7 @@ gif_load (struct frame *f, struct image if (rc == GIF_ERROR || gif->ImageCount <= 0) { image_error ("Error reading `%s'", img->spec, Qnil); - fn_DGifCloseFile (gif); + gif_close (gif, NULL); return 0; } @@ -7440,7 +7457,7 @@ gif_load (struct frame *f, struct image { image_error ("Invalid image number `%s' in image `%s'", image_number, img->spec); - fn_DGifCloseFile (gif); + gif_close (gif, NULL); return 0; } } @@ -7458,7 +7475,7 @@ gif_load (struct frame *f, struct image if (!check_image_size (f, width, height)) { image_error ("Invalid image size (see `max-image-size')", Qnil, Qnil); - fn_DGifCloseFile (gif); + gif_close (gif, NULL); return 0; } @@ -7476,7 +7493,7 @@ gif_load (struct frame *f, struct image && 0 <= subimg_left && subimg_left <= width - subimg_width)) { image_error ("Subimage does not fit in image", Qnil, Qnil); - fn_DGifCloseFile (gif); + gif_close (gif, NULL); return 0; } } @@ -7484,7 +7501,7 @@ gif_load (struct frame *f, struct image /* Create the X image and pixmap. */ if (!image_create_x_image_and_pixmap (f, img, width, height, 0, &ximg, 0)) { - fn_DGifCloseFile (gif); + gif_close (gif, NULL); return 0; } @@ -7655,7 +7672,18 @@ gif_load (struct frame *f, struct image Fcons (make_number (gif->ImageCount), img->lisp_data)); - fn_DGifCloseFile (gif); + if (gif_close (gif, &gif_err) == GIF_ERROR) + { +#if 5 <= GIFLIB_MAJOR + char *error_text = fn_GifErrorString (gif_err); + + if (error_text) + image_error ("Error closing `%s': %s", + img->spec, build_string (error_text)); +#else + image_error ("Error closing `%s'", img->spec); +#endif + } /* Maybe fill in the background field while we have ximg handy. */ if (NILP (image_spec_value (img->spec, QCbackground, NULL))) === modified file 'lisp/term/w32-win.el' --- lisp/term/w32-win.el 2014-04-11 07:02:28 +0000 +++ lisp/term/w32-win.el 2014-06-17 14:51:18 +0000 @@ -251,13 +251,16 @@ This returns an error if any Emacs frame ;; libraries according to the version of giflib we were ;; compiled against. (If we were compiled without GIF support, ;; libgif-version's value is -1.) - (if (>= libgif-version 50000) - ;; Yes, giflib 5.x uses 6 as the major version of the API, - ;; thus "libgif-6.dll" below (giflib 4.x used 5 as the - ;; major API version). - ;; giflib5.dll is from the lua-files project. - '(gif "libgif-6.dll" "giflib5.dll") - '(gif "libgif-5.dll" "giflib4.dll" "libungif4.dll" "libungif.dll")) + (if (>= libgif-version 50100) + ;; Yes, giflib 5.0 uses 6 as the major version of the API, + ;; and giflib 5.1 uses 7, thus "libgif-7.dll" and + ;; "libgif-6.dll" below (giflib 4.x used 5 as the major API + ;; version). giflib5.dll is from the lua-files project, + ;; and gif.dll is from luapower. + '(gif "libgif-7.dll") + (if (>= libgif-version 50000) + '(gif "libgif-6.dll" "giflib5.dll" "gif.dll") + '(gif "libgif-5.dll" "giflib4.dll" "libungif4.dll" "libungif.dll"))) '(svg "librsvg-2-2.dll") '(gdk-pixbuf "libgdk_pixbuf-2.0-0.dll") '(glib "libglib-2.0-0.dll")