bug#57755: 29.0.50; [PATCH] Fallout 816106b

bug#57755: 29.0.50; [PATCH] Fallout 816106b

[dick]

[-- Attachment #1: Type: text/plain, Size: 167 bytes --]


src/emacs -Q --eval \
"(let ((max-image-size 0)) \
  (find-file (plist-get \
	      (cdr (find-image (quote ((:file \"splash.svg\" :type svg))))) \
	      :file)))"


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Fallout-816106b.patch --]
[-- Type: text/x-diff, Size: 4677 bytes --]

From 8777430afbd57d0a055f7fbb57c6e576441c9ace Mon Sep 17 00:00:00 2001
From: dickmao <dick.r.chiang@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))))))
 
 \f
 ;;;; image-test-size
-- 
2.36.1


[-- Attachment #3: Type: text/plain, Size: 12897 bytes --]




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))

bug#57755: 29.0.50; [PATCH] Fallout 816106b

[dick]

[-- Attachment #1: Type: text/plain, Size: 24 bytes --]

Oh right, ming wong32.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Fallout-816106b.patch --]
[-- Type: text/x-diff, Size: 4827 bytes --]

From 644482da6e69c38b75b461c13a9eb404e014ca78 Mon Sep 17 00:00:00 2001
From: dickmao <dick.r.chiang@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))))))
 
 \f
 ;;;; image-test-size
-- 
2.36.1

bug#57755: 29.0.50; [PATCH] Fallout 816106b

[Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors]

dick <dick.r.chiang@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.

bug#57755: 29.0.50; [PATCH] Fallout 816106b

[Eli Zaretskii]

> Cc: 57755@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@gnu.org>
> 
> dick <dick.r.chiang@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.

bug#57755: 29.0.50; [PATCH] Fallout 816106b

[Lars Ingebrigtsen]

dick <dick.r.chiang@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.

bug#57755: 29.0.50; [PATCH] Fallout 816106b

[Lars Ingebrigtsen]

Po Lu <luangruo@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#57755: 29.0.50; [PATCH] Fallout 816106b

[Eli Zaretskii]

> Cc: 57755@debbugs.gnu.org
> From: Lars Ingebrigtsen <larsi@gnus.org>
> Date: Tue, 13 Sep 2022 13:57:59 +0200
> 
> dick <dick.r.chiang@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!

bug#57755: 29.0.50; [PATCH] Fallout 816106b

[Lars Ingebrigtsen]

Eli Zaretskii <eliz@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.

bug#57755: 29.0.50; [PATCH] Fallout 816106b

[Basil L. Contovounesios via Bug reports for GNU Emacs, the Swiss army knife of text editors]

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