GNU bug report logs - #57755
29.0.50; [PATCH] Fallout 816106b

Previous Next

Package: emacs;

Reported by: dick <dick.r.chiang <at> gmail.com>

Date: Mon, 12 Sep 2022 20:46:03 UTC

Severity: normal

Tags: patch

Found in version 29.0.50

Fixed in version 29.1

Done: Lars Ingebrigtsen <larsi <at> gnus.org>

Bug is archived. No further changes may be made.

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 57755 in the body.
You can then email your comments to 57755 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

View this report as an mbox folder, status mbox, maintainer mbox


Report forwarded to bug-gnu-emacs <at> gnu.org:
bug#57755; Package emacs. (Mon, 12 Sep 2022 20:46:03 GMT) Full text and rfc822 format available.

Acknowledgement sent to dick <dick.r.chiang <at> gmail.com>:
New bug report received and forwarded. Copy sent to bug-gnu-emacs <at> gnu.org. (Mon, 12 Sep 2022 20:46:03 GMT) Full text and rfc822 format available.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: dick <dick.r.chiang <at> gmail.com>
To: bug-gnu-emacs <bug-gnu-emacs <at> gnu.org>
Subject: 29.0.50; [PATCH] Fallout 816106b
Date: Mon, 12 Sep 2022 16:37:29 -0400
[Message part 1 (text/plain, inline)]
src/emacs -Q --eval \
"(let ((max-image-size 0)) \
  (find-file (plist-get \
	      (cdr (find-image (quote ((:file \"splash.svg\" :type svg))))) \
	      :file)))"

[0001-Fallout-816106b.patch (text/x-diff, inline)]
From 8777430afbd57d0a055f7fbb57c6e576441c9ace Mon Sep 17 00:00:00 2001
From: dickmao <dick.r.chiang <at> gmail.com>
Date: Mon, 12 Sep 2022 16:33:43 -0400
Subject: [PATCH] Fallout 816106b

* src/image.c (init_svg_functions, g_clear_error):
Disregard youthful indiscretions Bug#21641.
(svg_load_image): Avoid segfault and double reporting errors.
* test/manual/image-tests.el (image-tests-load-image/svg-too-big,
image-tests-load-image/svg-invalid): Test it.
---
 src/image.c                | 22 +++++++++++++---------
 test/manual/image-tests.el | 19 ++++++++++++++++++-
 2 files changed, 31 insertions(+), 10 deletions(-)

diff --git a/src/image.c b/src/image.c
index 549fe30ef7c..f0cecc1ffec 100644
--- a/src/image.c
+++ b/src/image.c
@@ -10907,7 +10907,6 @@ DEF_DLL_FN (int, gdk_pixbuf_get_bits_per_sample, (const GdkPixbuf *));
 DEF_DLL_FN (void, g_type_init, (void));
 #  endif
 DEF_DLL_FN (void, g_object_unref, (gpointer));
-DEF_DLL_FN (void, g_clear_error, (GError **));
 
 static bool
 init_svg_functions (void)
@@ -10967,7 +10966,6 @@ init_svg_functions (void)
   LOAD_DLL_FN (gobject, g_type_init);
 #  endif
   LOAD_DLL_FN (gobject, g_object_unref);
-  LOAD_DLL_FN (glib, g_clear_error);
 
   return 1;
 }
@@ -10983,7 +10981,6 @@ init_svg_functions (void)
 #  undef gdk_pixbuf_get_pixels
 #  undef gdk_pixbuf_get_rowstride
 #  undef gdk_pixbuf_get_width
-#  undef g_clear_error
 #  undef g_object_unref
 #  undef g_type_init
 #  if LIBRSVG_CHECK_VERSION (2, 52, 1)
@@ -11019,7 +11016,6 @@ init_svg_functions (void)
 #  define gdk_pixbuf_get_pixels fn_gdk_pixbuf_get_pixels
 #  define gdk_pixbuf_get_rowstride fn_gdk_pixbuf_get_rowstride
 #  define gdk_pixbuf_get_width fn_gdk_pixbuf_get_width
-#  define g_clear_error fn_g_clear_error
 #  define g_object_unref fn_g_object_unref
 #  if ! GLIB_CHECK_VERSION (2, 36, 0)
 #   define g_type_init fn_g_type_init
@@ -11353,7 +11349,7 @@ svg_load_image (struct frame *f, struct image *img, char *contents,
   if (! check_image_size (f, width, height))
     {
       image_size_error ();
-      goto rsvg_error;
+      goto done_error;
     }
 
   /* We are now done with the unmodified data.  */
@@ -11536,9 +11532,21 @@ svg_load_image (struct frame *f, struct image *img, char *contents,
     image_put_x_image (f, img, ximg, 0);
   }
 
+  eassume (err == NULL);
   return true;
 
  rsvg_error:
+  if (err == NULL)
+    image_error ("Error parsing SVG image");
+  else
+    {
+      image_error ("Error parsing SVG image: %s",
+		   call2 (intern ("string-trim-right"), build_string (err->message),
+			  Qnil));
+      g_error_free (err);
+    }
+
+ done_error:
   if (rsvg_handle)
     g_object_unref (rsvg_handle);
   if (wrapped_contents)
@@ -11547,10 +11555,6 @@ svg_load_image (struct frame *f, struct image *img, char *contents,
   if (css && !STRINGP (lcss))
     xfree (css);
 #endif
-  image_error ("Error parsing SVG image: %s",
-	       /* The -1 removes an extra newline.  */
-	       make_string (err->message, strlen (err->message) - 1));
-  g_clear_error (&err);
   return false;
 }
 
diff --git a/test/manual/image-tests.el b/test/manual/image-tests.el
index f867047d08e..76a22c25718 100644
--- a/test/manual/image-tests.el
+++ b/test/manual/image-tests.el
@@ -79,6 +79,21 @@ image-tests-make-load-image-test
 (image-tests-make-load-image-test 'xbm)
 (image-tests-make-load-image-test 'xpm)
 
+(ert-deftest image-tests-load-image/svg-too-big ()
+  (with-temp-buffer
+    (let* ((max-image-size 0)
+           (messages-buffer-name (buffer-name (current-buffer)))
+           (img (cdr (assq 'svg image-tests--images)))
+           (file (if (listp img)
+                     (plist-get (cdr img) :file)
+                   img)))
+      (save-excursion (find-file file))
+      (should (string-match-p "invalid image size" (buffer-string)))
+      ;; no annoying newlines
+      (should-not (string-match-p "^[ \t\n\r]+$" (buffer-string)))
+      ;; no annoying double error reporting
+      (should-not (string-match-p "error parsing" (buffer-string))))))
+
 (ert-deftest image-tests-load-image/svg-invalid ()
   (with-temp-buffer
     (let ((messages-buffer-name (buffer-name (current-buffer))))
@@ -90,7 +105,9 @@ image-tests-load-image/svg-invalid
                                              :type svg)))
         (redisplay))
       ;; librsvg error: "... Start tag expected, '<' not found [3 times]"
-      (should (string-match "[Ee]rror.+Start tag expected" (buffer-string))))))
+      (should (string-match-p "[Ee]rror.+Start tag expected" (buffer-string)))
+      ;; no annoying newlines
+      (should-not (string-match-p "^[ \t\n\r]+$" (buffer-string))))))
 
 
 ;;;; image-test-size
-- 
2.36.1

[Message part 3 (text/plain, inline)]


In Commercial Emacs 0.3.1snapshot c9ee64c in dev (upstream 29.0.50,
x86_64-pc-linux-gnu) built on dick
Repository revision: c9ee64c7a26563b203f5eab6b9898202cc79ae91
Repository branch: dev
Windowing system distributor 'The X.Org Foundation', version 11.0.12013000
System Description: Ubuntu 20.04.4 LTS

Configured using:
 'configure WERROR_CFLAGS=-Werror --prefix=/home/dick/.local
 --with-tree-sitter'

Configured features:
CAIRO DBUS FREETYPE GIF GLIB GMP GNUTLS GSETTINGS HARFBUZZ JPEG JSON
TREE_SITTER LCMS2 LIBSELINUX LIBXML2 MODULES NOTIFY INOTIFY PDUMPER PNG
RSVG SECCOMP SOUND THREADS TIFF TOOLKIT_SCROLL_BARS WEBP X11 XDBE XIM
XINPUT2 XPM GTK3 ZLIB

Important settings:
  value of $LANG: en_US.UTF-8
  locale-coding-system: utf-8-unix

Major mode: ELisp/l

Minor modes in effect:
  tree-sitter-lock-mode: t
  shell-dirtrack-mode: t
  bug-reference-prog-mode: t
  paredit-mode: t
  projectile-mode: t
  flx-ido-mode: t
  override-global-mode: t
  global-hl-line-mode: t
  hl-line-mode: t
  winner-mode: t
  tooltip-mode: t
  global-eldoc-mode: t
  eldoc-mode: t
  show-paren-mode: t
  mouse-wheel-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  column-number-mode: t
  line-number-mode: t
  transient-mark-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t

Load-path shadows:
/home/dick/gomacro-mode/gomacro-mode hides /home/dick/.emacs.d/elpa/gomacro-mode-20200326.1103/gomacro-mode
/home/dick/.emacs.d/elpa/go-rename-20190805.2101/go-rename hides /home/dick/.emacs.d/elpa/go-mode-1.6.0/go-rename
/home/dick/.emacs.d/elpa/go-guru-20181012.330/go-guru hides /home/dick/.emacs.d/elpa/go-mode-1.6.0/go-guru
/home/dick/org-gcal.el/org-gcal hides /home/dick/.emacs.d/elpa/org-gcal-0.3/org-gcal
/home/dick/.emacs.d/elpa/request-deferred-0.2.0/request-deferred hides /home/dick/.emacs.d/elpa/request-0.3.3/request-deferred
/home/dick/.emacs.d/elpa/chess-2.0.5/_pkg hides /home/dick/.local/share/emacs/site-lisp/_pkg
/home/dick/.emacs.d/elpa/chess-2.0.5/chess-pos hides /home/dick/.local/share/emacs/site-lisp/chess-pos
/home/dick/.emacs.d/elpa/chess-2.0.5/chess-module hides /home/dick/.local/share/emacs/site-lisp/chess-module
/home/dick/.emacs.d/elpa/chess-2.0.5/chess-ucb hides /home/dick/.local/share/emacs/site-lisp/chess-ucb
/home/dick/.emacs.d/elpa/chess-2.0.5/chess-scid hides /home/dick/.local/share/emacs/site-lisp/chess-scid
/home/dick/.emacs.d/elpa/chess-2.0.5/chess-puzzle hides /home/dick/.local/share/emacs/site-lisp/chess-puzzle
/home/dick/.emacs.d/elpa/chess-2.0.5/chess-irc hides /home/dick/.local/share/emacs/site-lisp/chess-irc
/home/dick/.emacs.d/elpa/chess-2.0.5/chess-network hides /home/dick/.local/share/emacs/site-lisp/chess-network
/home/dick/.emacs.d/elpa/chess-2.0.5/chess-autosave hides /home/dick/.local/share/emacs/site-lisp/chess-autosave
/home/dick/.emacs.d/elpa/chess-2.0.5/chess-engine hides /home/dick/.local/share/emacs/site-lisp/chess-engine
/home/dick/.emacs.d/elpa/chess-2.0.5/chess-tutorial hides /home/dick/.local/share/emacs/site-lisp/chess-tutorial
/home/dick/.emacs.d/elpa/chess-2.0.5/chess-german hides /home/dick/.local/share/emacs/site-lisp/chess-german
/home/dick/.emacs.d/elpa/chess-2.0.5/chess-file hides /home/dick/.local/share/emacs/site-lisp/chess-file
/home/dick/.emacs.d/elpa/chess-2.0.5/chess-random hides /home/dick/.local/share/emacs/site-lisp/chess-random
/home/dick/.emacs.d/elpa/chess-2.0.5/chess-stockfish hides /home/dick/.local/share/emacs/site-lisp/chess-stockfish
/home/dick/.emacs.d/elpa/chess-2.0.5/chess-pgn hides /home/dick/.local/share/emacs/site-lisp/chess-pgn
/home/dick/.emacs.d/elpa/chess-2.0.5/chess-kibitz hides /home/dick/.local/share/emacs/site-lisp/chess-kibitz
/home/dick/.emacs.d/elpa/chess-2.0.5/chess-eco hides /home/dick/.local/share/emacs/site-lisp/chess-eco
/home/dick/.emacs.d/elpa/chess-2.0.5/chess-display hides /home/dick/.local/share/emacs/site-lisp/chess-display
/home/dick/.emacs.d/elpa/chess-2.0.5/chess-var hides /home/dick/.local/share/emacs/site-lisp/chess-var
/home/dick/.emacs.d/elpa/chess-2.0.5/chess-test hides /home/dick/.local/share/emacs/site-lisp/chess-test
/home/dick/.emacs.d/elpa/chess-2.0.5/chess-ply hides /home/dick/.local/share/emacs/site-lisp/chess-ply
/home/dick/.emacs.d/elpa/chess-2.0.5/chess-message hides /home/dick/.local/share/emacs/site-lisp/chess-message
/home/dick/.emacs.d/elpa/chess-2.0.5/chess-ics1 hides /home/dick/.local/share/emacs/site-lisp/chess-ics1
/home/dick/.emacs.d/elpa/chess-2.0.5/chess-phalanx hides /home/dick/.local/share/emacs/site-lisp/chess-phalanx
/home/dick/.emacs.d/elpa/chess-2.0.5/chess-game hides /home/dick/.local/share/emacs/site-lisp/chess-game
/home/dick/.emacs.d/elpa/chess-2.0.5/chess-log hides /home/dick/.local/share/emacs/site-lisp/chess-log
/home/dick/.emacs.d/elpa/chess-2.0.5/chess-plain hides /home/dick/.local/share/emacs/site-lisp/chess-plain
/home/dick/.emacs.d/elpa/chess-2.0.5/chess-perft hides /home/dick/.local/share/emacs/site-lisp/chess-perft
/home/dick/.emacs.d/elpa/chess-2.0.5/chess-glaurung hides /home/dick/.local/share/emacs/site-lisp/chess-glaurung
/home/dick/.emacs.d/elpa/chess-2.0.5/chess-ai hides /home/dick/.local/share/emacs/site-lisp/chess-ai
/home/dick/.emacs.d/elpa/chess-2.0.5/chess-fruit hides /home/dick/.local/share/emacs/site-lisp/chess-fruit
/home/dick/.emacs.d/elpa/chess-2.0.5/chess-uci hides /home/dick/.local/share/emacs/site-lisp/chess-uci
/home/dick/.emacs.d/elpa/chess-2.0.5/chess-epd hides /home/dick/.local/share/emacs/site-lisp/chess-epd
/home/dick/.emacs.d/elpa/chess-2.0.5/chess-database hides /home/dick/.local/share/emacs/site-lisp/chess-database
/home/dick/.emacs.d/elpa/chess-2.0.5/chess-link hides /home/dick/.local/share/emacs/site-lisp/chess-link
/home/dick/.emacs.d/elpa/chess-2.0.5/chess-transport hides /home/dick/.local/share/emacs/site-lisp/chess-transport
/home/dick/.emacs.d/elpa/chess-2.0.5/chess-none hides /home/dick/.local/share/emacs/site-lisp/chess-none
/home/dick/.emacs.d/elpa/chess-2.0.5/chess-polyglot hides /home/dick/.local/share/emacs/site-lisp/chess-polyglot
/home/dick/.emacs.d/elpa/chess-2.0.5/chess-crafty hides /home/dick/.local/share/emacs/site-lisp/chess-crafty
/home/dick/.emacs.d/elpa/chess-2.0.5/chess-chat hides /home/dick/.local/share/emacs/site-lisp/chess-chat
/home/dick/.emacs.d/elpa/chess-2.0.5/chess hides /home/dick/.local/share/emacs/site-lisp/chess
/home/dick/.emacs.d/elpa/chess-2.0.5/chess-images hides /home/dick/.local/share/emacs/site-lisp/chess-images
/home/dick/.emacs.d/elpa/chess-2.0.5/chess-gnuchess hides /home/dick/.local/share/emacs/site-lisp/chess-gnuchess
/home/dick/.emacs.d/elpa/chess-2.0.5/chess-fen hides /home/dick/.local/share/emacs/site-lisp/chess-fen
/home/dick/.emacs.d/elpa/chess-2.0.5/chess-ics hides /home/dick/.local/share/emacs/site-lisp/chess-ics
/home/dick/.emacs.d/elpa/chess-2.0.5/chess-ics2 hides /home/dick/.local/share/emacs/site-lisp/chess-ics2
/home/dick/.emacs.d/elpa/chess-2.0.5/chess-common hides /home/dick/.local/share/emacs/site-lisp/chess-common
/home/dick/.emacs.d/elpa/chess-2.0.5/chess-input hides /home/dick/.local/share/emacs/site-lisp/chess-input
/home/dick/.emacs.d/elpa/chess-2.0.5/chess-announce hides /home/dick/.local/share/emacs/site-lisp/chess-announce
/home/dick/.emacs.d/elpa/chess-2.0.5/chess-clock hides /home/dick/.local/share/emacs/site-lisp/chess-clock
/home/dick/.emacs.d/elpa/chess-2.0.5/chess-sound hides /home/dick/.local/share/emacs/site-lisp/chess-sound
/home/dick/.emacs.d/elpa/chess-2.0.5/chess-sjeng hides /home/dick/.local/share/emacs/site-lisp/chess-sjeng
/home/dick/.emacs.d/elpa/chess-2.0.5/chess-algebraic hides /home/dick/.local/share/emacs/site-lisp/chess-algebraic
/home/dick/.emacs.d/elpa/transient-0.3.7snapshot/transient hides /home/dick/.local/share/emacs/0.3.1/lisp/transient

Features:
(shadow sort footnote mail-extr gnus-msg emacsbug vc sh-script
executable blamer a misearch multi-isearch tree-sitter tramp-archive
tramp-gvfs tramp-cache time-stamp zeroconf tramp tramp-loaddefs trampver
tramp-integration cus-start files-x tramp-compat shell ls-lisp shr-color
textsec uni-scripts idna-mapping ucs-normalize uni-confusable
textsec-check url-http url-gw network-stream nsm url-cache url-auth
vc-git diff-mode vc-dispatcher bug-reference org-element avl-tree ol-eww
eww xdg url-queue ol-rmail ol-mhe ol-irc ol-info ol-gnus nnselect
gnus-art mm-uu mml2015 mm-view mml-smime smime gnutls dig gnus-sum
gnus-group mm-url gnus-undo gnus-start gnus-dbus dbus gnus-cloud nnimap
nnmail mail-source utf7 nnoo parse-time gnus-spec gnus-int gnus-range
message sendmail yank-media rfc822 mml mml-sec epa epg rfc6068
epg-config mm-decode mm-bodies mm-encode mail-parse rfc2231 rfc2047
rfc2045 ietf-drums mailabbrev gmm-utils gnus-win ol-docview doc-view
jka-compr image-mode exif dired-x dired dired-loaddefs ol-bibtex ol-bbdb
ol-w3m ol-doi org-link-doi org-tempo tempo org org-macro org-footnote
org-pcomplete pcomplete org-list org-faces org-entities org-version ob-R
ob-emacs-lisp ob-ein ein-cell ein-shared-output ein-output-area shr
pixel-fill kinsoku url-file puny svg dom xml ein-kernel ein-ipdb
ein-query ein-events ein-websocket websocket bindat ein-node ewoc
ein-log ein-classes ein-core request mailheader anaphora ein ein-utils
deferred cc-mode cc-fonts cc-guess cc-menus cc-cmds cc-styles cc-align
cc-engine cc-vars cc-defs ob ob-tangle org-src ob-ref ob-lob ob-table
ob-exp ob-comint ob-core ob-eval org-table oc-basic bibtex iso8601 ol
org-keys oc org-compat org-macs org-loaddefs cal-menu calendar
cal-loaddefs paredit-ext paredit inf-ruby ruby-mode smie company pcase
haskell-interactive-mode haskell-presentation-mode haskell-process
haskell-session haskell-compile haskell-mode haskell-cabal haskell-utils
haskell-font-lock haskell-indentation haskell-string
haskell-sort-imports haskell-lexeme haskell-align-imports
haskell-complete-module haskell-ghc-support noutline outline
flymake-proc flymake warnings etags fileloop generator dabbrev
haskell-customize hydra lv use-package-ensure solarized-theme
solarized-definitions projectile lisp-mnt ibuf-ext ibuffer
ibuffer-loaddefs thingatpt magit-autorevert autorevert filenotify
magit-git magit-base magit-section format-spec crm dash rx compat-27
compat-26 compat grep compile comint ansi-color gnus nnheader range
mail-utils mm-util mail-prsvr gnus-util text-property-search time-date
flx-ido flx google-translate-default-ui google-translate-core-ui
facemenu color ido google-translate-core google-translate-tk
google-translate-backend use-package-bind-key bind-key auto-complete
easy-mmode advice edmacro kmacro popup cus-edit pp cus-load icons
wid-edit emms-player-mplayer emms-player-simple emms emms-compat
cl-extra help-mode xref project use-package-core derived hl-line winner
ring acm-autoloads debbugs-autoloads eglot-autoloads
elpaso-disc-autoloads elpaso-autoloads find-func finder-inf
go-mode-autoloads json-reformat-autoloads json-snatcher-autoloads
lsp-bridge-autoloads magit-autoloads posframe-autoloads
projectile-autoloads markdown-mode-autoloads rust-mode-autoloads
sml-mode-autoloads epl-autoloads tornado-template-mode-autoloads
typescript-mode-autoloads request-autoloads info wordnut-autoloads
yasnippet-autoloads package browse-url url url-proxy url-privacy
url-expand url-methods url-history url-cookie generate-lisp-file
url-domsuf url-util mailcap url-handlers url-parse auth-source cl-seq
eieio eieio-core cl-macs password-cache json subr-x map byte-opt gv
bytecomp byte-compile cconv cldefs url-vars cl-loaddefs cl-lib rmc
iso-transl tooltip eldoc paren electric uniquify ediff-hook vc-hooks
lisp-float-type elisp-mode mwheel term/x-win x-win term/common-win x-dnd
tool-bar dnd fontset image regexp-opt fringe tabulated-list replace
newcomment text-mode lisp-mode prog-mode register page tab-bar menu-bar
rfn-eshadow isearch easymenu timer select scroll-bar mouse jit-lock
font-lock syntax font-core term/tty-colors frame minibuffer nadvice seq
simple cl-generic indonesian philippine cham georgian utf-8-lang
misc-lang vietnamese tibetan thai tai-viet lao korean japanese eucjp-ms
cp51932 hebrew greek romanian slovak czech european ethiopic indian
cyrillic chinese composite emoji-zwj charscript charprop case-table
epa-hook jka-cmpr-hook help abbrev obarray oclosure cl-preloaded button
loaddefs faces cus-face macroexp files window text-properties overlay
sha1 md5 base64 format env code-pages mule custom widget keymap
hashtable-print-readable backquote threads dbusbind inotify lcms2
dynamic-setting system-font-setting font-render-setting cairo
move-toolbar gtk x-toolkit xinput2 x multi-tty make-network-process
emacs)

Memory information:
((conses 16 902810 21728)
 (symbols 48 39295 0)
 (strings 32 178416 20945)
 (string-bytes 1 5055753)
 (vectors 16 106644)
 (vector-slots 8 1967351 89879)
 (floats 8 454 288)
 (intervals 56 41865 0)
 (buffers 1008 16))

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#57755; Package emacs. (Tue, 13 Sep 2022 07:28:02 GMT) Full text and rfc822 format available.

Message #8 received at 57755 <at> debbugs.gnu.org (full text, mbox):

From: dick <dick.r.chiang <at> gmail.com>
To: 57755 <at> debbugs.gnu.org
Subject: Re: bug#57755: 29.0.50; [PATCH] Fallout 816106b
Date: Mon, 12 Sep 2022 17:16:02 -0400
[Message part 1 (text/plain, inline)]
Oh right, ming wong32.

[0001-Fallout-816106b.patch (text/x-diff, inline)]
From 644482da6e69c38b75b461c13a9eb404e014ca78 Mon Sep 17 00:00:00 2001
From: dickmao <dick.r.chiang <at> gmail.com>
Date: Mon, 12 Sep 2022 17:08:09 -0400
Subject: [PATCH] Fallout 816106b

* src/image.c (init_svg_functions, g_clear_error):
Disregard youthful indiscretions Bug#21641.
(svg_load_image): Avoid segfault and double reporting errors.
* test/manual/image-tests.el (image-tests-load-image/svg-too-big,
image-tests-load-image/svg-invalid): Test it.
---
 src/image.c                | 26 +++++++++++++++++---------
 test/manual/image-tests.el | 19 ++++++++++++++++++-
 2 files changed, 35 insertions(+), 10 deletions(-)

diff --git a/src/image.c b/src/image.c
index 549fe30ef7c..87ddb19b37e 100644
--- a/src/image.c
+++ b/src/image.c
@@ -10907,7 +10907,7 @@ DEF_DLL_FN (int, gdk_pixbuf_get_bits_per_sample, (const GdkPixbuf *));
 DEF_DLL_FN (void, g_type_init, (void));
 #  endif
 DEF_DLL_FN (void, g_object_unref, (gpointer));
-DEF_DLL_FN (void, g_clear_error, (GError **));
+DEF_DLL_FN (void, g_error_free, (GError *));
 
 static bool
 init_svg_functions (void)
@@ -10967,7 +10967,7 @@ init_svg_functions (void)
   LOAD_DLL_FN (gobject, g_type_init);
 #  endif
   LOAD_DLL_FN (gobject, g_object_unref);
-  LOAD_DLL_FN (glib, g_clear_error);
+  LOAD_DLL_FN (glib, g_error_free);
 
   return 1;
 }
@@ -10983,7 +10983,7 @@ init_svg_functions (void)
 #  undef gdk_pixbuf_get_pixels
 #  undef gdk_pixbuf_get_rowstride
 #  undef gdk_pixbuf_get_width
-#  undef g_clear_error
+#  undef g_error_free
 #  undef g_object_unref
 #  undef g_type_init
 #  if LIBRSVG_CHECK_VERSION (2, 52, 1)
@@ -11019,7 +11019,7 @@ init_svg_functions (void)
 #  define gdk_pixbuf_get_pixels fn_gdk_pixbuf_get_pixels
 #  define gdk_pixbuf_get_rowstride fn_gdk_pixbuf_get_rowstride
 #  define gdk_pixbuf_get_width fn_gdk_pixbuf_get_width
-#  define g_clear_error fn_g_clear_error
+#  define g_error_free fn_g_error_free
 #  define g_object_unref fn_g_object_unref
 #  if ! GLIB_CHECK_VERSION (2, 36, 0)
 #   define g_type_init fn_g_type_init
@@ -11353,7 +11353,7 @@ svg_load_image (struct frame *f, struct image *img, char *contents,
   if (! check_image_size (f, width, height))
     {
       image_size_error ();
-      goto rsvg_error;
+      goto done_error;
     }
 
   /* We are now done with the unmodified data.  */
@@ -11536,9 +11536,21 @@ svg_load_image (struct frame *f, struct image *img, char *contents,
     image_put_x_image (f, img, ximg, 0);
   }
 
+  eassume (err == NULL);
   return true;
 
  rsvg_error:
+  if (err == NULL)
+    image_error ("Error parsing SVG image");
+  else
+    {
+      image_error ("Error parsing SVG image: %s",
+		   call2 (intern ("string-trim-right"), build_string (err->message),
+			  Qnil));
+      g_error_free (err);
+    }
+
+ done_error:
   if (rsvg_handle)
     g_object_unref (rsvg_handle);
   if (wrapped_contents)
@@ -11547,10 +11559,6 @@ svg_load_image (struct frame *f, struct image *img, char *contents,
   if (css && !STRINGP (lcss))
     xfree (css);
 #endif
-  image_error ("Error parsing SVG image: %s",
-	       /* The -1 removes an extra newline.  */
-	       make_string (err->message, strlen (err->message) - 1));
-  g_clear_error (&err);
   return false;
 }
 
diff --git a/test/manual/image-tests.el b/test/manual/image-tests.el
index f867047d08e..76a22c25718 100644
--- a/test/manual/image-tests.el
+++ b/test/manual/image-tests.el
@@ -79,6 +79,21 @@ image-tests-make-load-image-test
 (image-tests-make-load-image-test 'xbm)
 (image-tests-make-load-image-test 'xpm)
 
+(ert-deftest image-tests-load-image/svg-too-big ()
+  (with-temp-buffer
+    (let* ((max-image-size 0)
+           (messages-buffer-name (buffer-name (current-buffer)))
+           (img (cdr (assq 'svg image-tests--images)))
+           (file (if (listp img)
+                     (plist-get (cdr img) :file)
+                   img)))
+      (save-excursion (find-file file))
+      (should (string-match-p "invalid image size" (buffer-string)))
+      ;; no annoying newlines
+      (should-not (string-match-p "^[ \t\n\r]+$" (buffer-string)))
+      ;; no annoying double error reporting
+      (should-not (string-match-p "error parsing" (buffer-string))))))
+
 (ert-deftest image-tests-load-image/svg-invalid ()
   (with-temp-buffer
     (let ((messages-buffer-name (buffer-name (current-buffer))))
@@ -90,7 +105,9 @@ image-tests-load-image/svg-invalid
                                              :type svg)))
         (redisplay))
       ;; librsvg error: "... Start tag expected, '<' not found [3 times]"
-      (should (string-match "[Ee]rror.+Start tag expected" (buffer-string))))))
+      (should (string-match-p "[Ee]rror.+Start tag expected" (buffer-string)))
+      ;; no annoying newlines
+      (should-not (string-match-p "^[ \t\n\r]+$" (buffer-string))))))
 
 
 ;;;; image-test-size
-- 
2.36.1


Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#57755; Package emacs. (Tue, 13 Sep 2022 07:34:02 GMT) Full text and rfc822 format available.

Message #11 received at 57755 <at> debbugs.gnu.org (full text, mbox):

From: Po Lu <luangruo <at> yahoo.com>
To: dick <dick.r.chiang <at> gmail.com>
Cc: 57755 <at> debbugs.gnu.org
Subject: Re: bug#57755: 29.0.50; [PATCH] Fallout 816106b
Date: Tue, 13 Sep 2022 15:32:56 +0800
dick <dick.r.chiang <at> gmail.com> writes:

> +		   call2 (intern ("string-trim-right"), build_string (err->message),

What coding system is err->message?

And calling intern with a static string is IMHO an anti-pattern that
should not be introduced further into our code.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#57755; Package emacs. (Tue, 13 Sep 2022 11:57:01 GMT) Full text and rfc822 format available.

Message #14 received at 57755 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Po Lu <luangruo <at> yahoo.com>
Cc: 57755 <at> debbugs.gnu.org, dick.r.chiang <at> gmail.com
Subject: Re: bug#57755: 29.0.50; [PATCH] Fallout 816106b
Date: Tue, 13 Sep 2022 14:56:23 +0300
> Cc: 57755 <at> debbugs.gnu.org
> Date: Tue, 13 Sep 2022 15:32:56 +0800
> From:  Po Lu via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs <at> gnu.org>
> 
> dick <dick.r.chiang <at> gmail.com> writes:
> 
> > +		   call2 (intern ("string-trim-right"), build_string (err->message),
> 
> What coding system is err->message?

That's a good question, but the same question applies to the original
code, because build_string is just a thin wrapper around make_string.

(My guess would be that the message is either US ASCII or UTF-8, given
its type in glib.  But if we want to be more defensive about that, it
should be a separate change.)

> And calling intern with a static string is IMHO an anti-pattern that
> should not be introduced further into our code.

I actually don't understand the rationale for calling
string-trim-right, not at all.  Why do we need such heavy artillery to
just remove one newline character?

As for replacing of g_clear_error with g_error_free: that's just
syntactic sugar, right?  It is not a mistake to use g_clear_error
there.  I see no reason to make such a replacement.

AFAIU, the useful part of this patch is that it avoids dereferencing
NULL.  That part should go in, but I don't think we want the rest.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#57755; Package emacs. (Tue, 13 Sep 2022 11:59:02 GMT) Full text and rfc822 format available.

Message #17 received at 57755 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: dick <dick.r.chiang <at> gmail.com>
Cc: 57755 <at> debbugs.gnu.org
Subject: Re: bug#57755: 29.0.50; [PATCH] Fallout 816106b
Date: Tue, 13 Sep 2022 13:57:59 +0200
dick <dick.r.chiang <at> gmail.com> writes:

> * src/image.c (init_svg_functions, g_clear_error):
> Disregard youthful indiscretions Bug#21641.
> (svg_load_image): Avoid segfault and double reporting errors.
> * test/manual/image-tests.el (image-tests-load-image/svg-too-big,
> image-tests-load-image/svg-invalid): Test it.

Thanks; pushed to Emacs 29.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#57755; Package emacs. (Tue, 13 Sep 2022 12:00:02 GMT) Full text and rfc822 format available.

Message #20 received at 57755 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Po Lu <luangruo <at> yahoo.com>
Cc: 57755 <at> debbugs.gnu.org, dick <dick.r.chiang <at> gmail.com>
Subject: Re: bug#57755: 29.0.50; [PATCH] Fallout 816106b
Date: Tue, 13 Sep 2022 13:59:00 +0200
Po Lu <luangruo <at> yahoo.com> writes:

>> + call2 (intern ("string-trim-right"), build_string (err->message),
>
> What coding system is err->message?

We had a make_string here before, so that's not really a change.  But,
yes, it should probably do some encoding here.

> And calling intern with a static string is IMHO an anti-pattern that
> should not be introduced further into our code.

Fixed.




bug marked as fixed in version 29.1, send any further explanations to 57755 <at> debbugs.gnu.org and dick <dick.r.chiang <at> gmail.com> Request was from Lars Ingebrigtsen <larsi <at> gnus.org> to control <at> debbugs.gnu.org. (Tue, 13 Sep 2022 12:00:02 GMT) Full text and rfc822 format available.

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#57755; Package emacs. (Tue, 13 Sep 2022 12:12:01 GMT) Full text and rfc822 format available.

Message #25 received at 57755 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: 57755 <at> debbugs.gnu.org, dick.r.chiang <at> gmail.com
Subject: Re: bug#57755: 29.0.50; [PATCH] Fallout 816106b
Date: Tue, 13 Sep 2022 15:11:11 +0300
> Cc: 57755 <at> debbugs.gnu.org
> From: Lars Ingebrigtsen <larsi <at> gnus.org>
> Date: Tue, 13 Sep 2022 13:57:59 +0200
> 
> dick <dick.r.chiang <at> gmail.com> writes:
> 
> > * src/image.c (init_svg_functions, g_clear_error):
> > Disregard youthful indiscretions Bug#21641.
> > (svg_load_image): Avoid segfault and double reporting errors.
> > * test/manual/image-tests.el (image-tests-load-image/svg-too-big,
> > image-tests-load-image/svg-invalid): Test it.
> 
> Thanks; pushed to Emacs 29.

I think calling a Lisp function to remove trailing whitespace from a C
string is butt-ugly.  Why not fire a Python or Guile interpreter to do
that?

Yuck!




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#57755; Package emacs. (Tue, 13 Sep 2022 12:36:02 GMT) Full text and rfc822 format available.

Message #28 received at 57755 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 57755 <at> debbugs.gnu.org, dick.r.chiang <at> gmail.com
Subject: Re: bug#57755: 29.0.50; [PATCH] Fallout 816106b
Date: Tue, 13 Sep 2022 14:35:27 +0200
Eli Zaretskii <eliz <at> gnu.org> writes:

> I think calling a Lisp function to remove trailing whitespace from a C
> string is butt-ugly.  Why not fire a Python or Guile interpreter to do
> that?

Sure, why not?  I'll have a patch ready in a few minutes.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#57755; Package emacs. (Tue, 13 Sep 2022 12:49:01 GMT) Full text and rfc822 format available.

Message #31 received at 57755 <at> debbugs.gnu.org (full text, mbox):

From: "Basil L. Contovounesios" <contovob <at> tcd.ie>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: Lars Ingebrigtsen <larsi <at> gnus.org>, 57755 <at> debbugs.gnu.org,
 dick.r.chiang <at> gmail.com
Subject: Re: bug#57755: 29.0.50; [PATCH] Fallout 816106b
Date: Tue, 13 Sep 2022 15:48:38 +0300
Eli Zaretskii [2022-09-13 15:11 +0300] wrote:

> I think calling a Lisp function to remove trailing whitespace from a C
> string is butt-ugly.  Why not fire a Python or Guile interpreter to do
> that?
>
> Yuck!

I find using butterflies about as efficient, without requiring any extra
autoconfiscation.

-- 
Basil




bug archived. Request was from Debbugs Internal Request <help-debbugs <at> gnu.org> to internal_control <at> debbugs.gnu.org. (Wed, 12 Oct 2022 11:24:08 GMT) Full text and rfc822 format available.

This bug report was last modified 1 year and 169 days ago.

Previous Next


GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.