unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#58028: 29.0.50; Ensures no leakage of glyph_matrix
@ 2022-09-23 15:22 Manuel Giraud
  2022-09-23 15:49 ` Eli Zaretskii
  0 siblings, 1 reply; 15+ messages in thread
From: Manuel Giraud @ 2022-09-23 15:22 UTC (permalink / raw)
  To: 58028

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


Hi,

Maybe this problem never triggers.  It is a « better be safe » kind of
patch.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Ensures-no-leakage-of-glyph_matrix.patch --]
[-- Type: text/x-patch, Size: 1050 bytes --]

From f539c3b198201f8f33f02b2ff0b595b205e5364c Mon Sep 17 00:00:00 2001
From: Manuel Giraud <manuel@ledu-giraud.fr>
Date: Fri, 23 Sep 2022 17:14:44 +0200
Subject: [PATCH] Ensures no leakage of glyph_matrix

; * src/dispnew.c (allocate_matrices_for_window_redisplay): Ensures no
leakage of glyph_matrix.
---
 src/dispnew.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/dispnew.c b/src/dispnew.c
index b786f0f1ba..5659b3d4fc 100644
--- a/src/dispnew.c
+++ b/src/dispnew.c
@@ -1808,10 +1808,10 @@ allocate_matrices_for_window_redisplay (struct window *w)
 
 	  /* If matrices are not yet allocated, allocate them now.  */
 	  if (w->desired_matrix == NULL)
-	    {
-	      w->desired_matrix = new_glyph_matrix (NULL);
-	      w->current_matrix = new_glyph_matrix (NULL);
-	    }
+	    w->desired_matrix = new_glyph_matrix (NULL);
+
+	  if (w->current_matrix == NULL)
+	    w->current_matrix = new_glyph_matrix (NULL);
 
 	  dim.width = required_matrix_width (w);
 	  dim.height = required_matrix_height (w);
-- 
2.37.3


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



In GNU Emacs 29.0.50 (build 2, x86_64-unknown-openbsd7.2, cairo version
 1.17.6) of 2022-09-23 built on elite.giraud
Repository revision: 94380420e2cba7e2821b22fd8a46cd5b04c985ef
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.12101004
System Description: OpenBSD elite.giraud 7.2 GENERIC.MP#739 amd64

Configured using:
 'configure --prefix=/home/manuel/emacs --bindir=/home/manuel/bin
 --with-x-toolkit=no --without-sound --without-compress-install
 CPPFLAGS=-I/usr/local/include LDFLAGS=-L/usr/local/lib'

Configured features:
CAIRO DBUS FREETYPE GIF GLIB GMP GNUTLS GSETTINGS HARFBUZZ JPEG JSON
LCMS2 LIBOTF LIBXML2 MODULES NOTIFY KQUEUE OLDXMENU PDUMPER PNG RSVG
SQLITE3 THREADS TIFF WEBP X11 XDBE XIM XINPUT2 XPM ZLIB

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

Major mode: Dired by name

Minor modes in effect:
  gnus-dired-mode: t
  global-git-commit-mode: t
  magit-auto-revert-mode: t
  icomplete-mode: t
  display-time-mode: t
  display-battery-mode: t
  shell-dirtrack-mode: t
  global-so-long-mode: t
  repeat-mode: t
  global-eldoc-mode: t
  show-paren-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  buffer-read-only: t
  line-number-mode: t
  indent-tabs-mode: t
  transient-mark-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t

Load-path shadows:
/home/manuel/.emacs.d/elpa/transient-20220918.2101/transient hides /home/manuel/emacs/share/emacs/29.0.50/lisp/transient

Features:
(shadow emacsbug whitespace gnus-fun emoji-labels emoji multisession
sqlite ibuf-ext ibuffer ibuffer-loaddefs ispell magit-patch
magit-subtree magit-gitignore magit-ediff mailalias dabbrev cl-print
vc-src vc-sccs vc-cvs vc-rcs log-view vc boxquote-autoloads ediff
ediff-merg ediff-mult ediff-wind ediff-diff ediff-help ediff-init
ediff-util let-alist rust-utils rust-mode rust-rustfmt rust-playpen
rust-compile rust-cargo loaddefs-gen tar-mode finder-inf
display-line-numbers mouse-copy mouse-drag vc-hg vc-bzr gnus-dired
tramp-sh tramp-cache time-stamp sort gnus-cite mail-extr textsec
uni-scripts idna-mapping ucs-normalize uni-confusable textsec-check
gnus-async gnus-bcklg qp gnus-ml gnus-topic mm-archive url-http url-gw
url-cache url-auth utf-7 imap rfc2104 nndoc nndraft nnmh network-stream
nsm nnfolder nnml gnus-agent gnus-srvr gnus-score score-mode nnvirtual
nntp gnus-cache nnrss mm-url w3m doc-view jka-compr image-mode wallpaper
xdg exif w3m-hist w3m-fb bookmark-w3m w3m-ems w3m-favicon w3m-image
tab-line w3m-proc w3m-util shortdoc help-fns radix-tree misearch
multi-isearch find-dired ffap magit-extras face-remap magit-bookmark
magit-submodule magit-obsolete magit-blame magit-stash magit-reflog
magit-bisect magit-push magit-pull magit-fetch magit-clone magit-remote
magit-commit magit-sequence magit-notes magit-worktree magit-tag
magit-merge magit-branch magit-reset magit-files magit-refs magit-status
magit magit-repos magit-apply magit-wip magit-log which-func imenu
magit-diff smerge-mode diff git-commit log-edit pcvs-util add-log
magit-core magit-autorevert autorevert filenotify magit-margin
magit-transient magit-process with-editor magit-mode transient magit-git
magit-base magit-section dash compat-27 compat-26 compat compat-macs
vc-git diff-mode bug-reference pulse vc-dispatcher vc-svn pcmpl-unix
sh-script smie executable paredit edmacro icomplete time battery
exwm-randr xcb-randr exwm-config exwm exwm-input xcb-keysyms xcb-xkb
exwm-manage exwm-floating xcb-cursor xcb-render exwm-layout
exwm-workspace exwm-core xcb-ewmh xcb-icccm xcb xcb-xproto xcb-types
xcb-debug kmacro server stimmung-themes modus-operandi-theme
modus-themes ytdious osm mingus libmpdee reporter edebug debug backtrace
transmission diary-lib diary-loaddefs color calc-bin calc-ext calc
calc-loaddefs rect calc-macs w3m-load mu4e mu4e-org mu4e-main mu4e-view
mu4e-headers mu4e-compose mu4e-draft mu4e-actions smtpmail mu4e-search
mu4e-lists mu4e-bookmarks mu4e-mark mu4e-message flow-fill mule-util
hl-line mu4e-contacts mu4e-update mu4e-folders mu4e-server mu4e-context
mu4e-vars mu4e-helpers mu4e-config bookmark ido supercite regi
ebdb-message ebdb-gnus gnus-msg gnus-art mm-uu mml2015 mm-view mml-smime
smime gnutls dig gnus-sum shr pixel-fill kinsoku url-file svg dom
gnus-group gnus-undo gnus-start gnus-dbus gnus-cloud nnimap nnmail
mail-source utf7 nnoo gnus-spec gnus-int gnus-range message sendmail
yank-media puny rfc822 mml mml-sec epa epg rfc6068 epg-config mm-decode
mm-bodies mm-encode mail-parse rfc2231 rfc2047 rfc2045 ietf-drums
gmm-utils mailheader gnus-win gnus nnheader gnus-util mail-utils range
mm-util mail-prsvr ebdb-mua ebdb-com crm ebdb-format ebdb mailabbrev
eieio-opt speedbar ezimage dframe eieio-base pcase timezone org ob
ob-tangle ob-ref ob-lob ob-table ob-exp org-macro org-footnote org-src
ob-comint org-pcomplete org-list org-faces org-entities org-version
ob-emacs-lisp ob-core ob-eval org-table oc-basic bibtex ol org-keys oc
org-compat org-macs org-loaddefs find-func cal-menu calendar
cal-loaddefs visual-basic-mode cl web-mode disp-table erlang-start
smart-tabs-mode skeleton cc-mode cc-fonts cc-guess cc-menus cc-cmds
cc-styles cc-align cc-engine cc-vars cc-defs slime-asdf grep slime-tramp
tramp tramp-loaddefs trampver tramp-integration cus-edit cus-load
wid-edit files-x tramp-compat rx shell pcomplete parse-time iso8601
time-date ls-lisp format-spec slime-fancy slime-indentation
slime-cl-indent cl-indent slime-trace-dialog slime-fontifying-fu
slime-package-fu slime-references slime-compiler-notes-tree
slime-scratch slime-presentations advice bridge slime-macrostep
macrostep slime-mdot-fu slime-enclosing-context slime-fuzzy
slime-fancy-trace slime-fancy-inspector slime-c-p-c
slime-editing-commands slime-autodoc slime-repl elp slime-parse slime
derived cl-extra help-mode lisp-mnt gud apropos compile
text-property-search etags fileloop generator xref project arc-mode
archive-mode noutline outline icons pp comint osc ansi-color ring
hyperspec thingatpt slime-autoloads dired-aux dired-x dired
dired-loaddefs so-long notifications dbus xml repeat easy-mmode
auctex-autoloads tex-site debbugs-autoloads hyperbole-autoloads
magit-autoloads git-commit-autoloads magit-section-autoloads
dash-autoloads paredit-autoloads rust-mode-autoloads
stimmung-themes-autoloads transient-autoloads with-editor-autoloads info
compat-autoloads ytdious-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 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 kqueue lcms2
dynamic-setting system-font-setting font-render-setting cairo xinput2 x
multi-tty make-network-process emacs)

Memory information:
((conses 16 3026488 254443)
 (symbols 48 77071 8)
 (strings 32 521068 23385)
 (string-bytes 1 16133683)
 (vectors 16 224942)
 (vector-slots 8 3660420 266611)
 (floats 8 765 650)
 (intervals 56 262421 5119)
 (buffers 1000 108))

-- 
Manuel Giraud

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* bug#58028: 29.0.50; Ensures no leakage of glyph_matrix
  2022-09-23 15:22 bug#58028: 29.0.50; Ensures no leakage of glyph_matrix Manuel Giraud
@ 2022-09-23 15:49 ` Eli Zaretskii
  2022-09-23 15:59   ` Manuel Giraud
  0 siblings, 1 reply; 15+ messages in thread
From: Eli Zaretskii @ 2022-09-23 15:49 UTC (permalink / raw)
  To: Manuel Giraud; +Cc: 58028

> From: Manuel Giraud <manuel@ledu-giraud.fr>
> Date: Fri, 23 Sep 2022 17:22:07 +0200
> 
> Maybe this problem never triggers.  It is a « better be safe » kind of
> patch.

Please add an assertion there, to catch the case that previously was
handled by testing only w->desired_matrix.  It might be that what you
think is a memory-leak bug is in fact a feature.

Thanks.





^ permalink raw reply	[flat|nested] 15+ messages in thread

* bug#58028: 29.0.50; Ensures no leakage of glyph_matrix
  2022-09-23 15:49 ` Eli Zaretskii
@ 2022-09-23 15:59   ` Manuel Giraud
  2022-09-23 17:43     ` Eli Zaretskii
  0 siblings, 1 reply; 15+ messages in thread
From: Manuel Giraud @ 2022-09-23 15:59 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 58028

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Manuel Giraud <manuel@ledu-giraud.fr>
>> Date: Fri, 23 Sep 2022 17:22:07 +0200
>> 
>> Maybe this problem never triggers.  It is a « better be safe » kind of
>> patch.
>
> Please add an assertion there, to catch the case that previously was
> handled by testing only w->desired_matrix.

I do not understand what you mean here.

> It might be that what you think is a memory-leak bug is in fact a
> feature.

Yes, you're right (but I have a hard time to imagine how).  Maybe this
patch could just be ignored.

Thanks.
-- 
Manuel Giraud





^ permalink raw reply	[flat|nested] 15+ messages in thread

* bug#58028: 29.0.50; Ensures no leakage of glyph_matrix
  2022-09-23 15:59   ` Manuel Giraud
@ 2022-09-23 17:43     ` Eli Zaretskii
  2022-09-23 20:43       ` Manuel Giraud
  0 siblings, 1 reply; 15+ messages in thread
From: Eli Zaretskii @ 2022-09-23 17:43 UTC (permalink / raw)
  To: Manuel Giraud; +Cc: 58028

> From: Manuel Giraud <manuel@ledu-giraud.fr>
> Cc: 58028@debbugs.gnu.org
> Date: Fri, 23 Sep 2022 17:59:48 +0200
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> From: Manuel Giraud <manuel@ledu-giraud.fr>
> >> Date: Fri, 23 Sep 2022 17:22:07 +0200
> >> 
> >> Maybe this problem never triggers.  It is a « better be safe » kind of
> >> patch.
> >
> > Please add an assertion there, to catch the case that previously was
> > handled by testing only w->desired_matrix.
> 
> I do not understand what you mean here.

I mean the assertion that triggers when w->desired_matrix is NULL, but
w->current_matrix isn't.





^ permalink raw reply	[flat|nested] 15+ messages in thread

* bug#58028: 29.0.50; Ensures no leakage of glyph_matrix
  2022-09-23 17:43     ` Eli Zaretskii
@ 2022-09-23 20:43       ` Manuel Giraud
  2022-09-24  0:26         ` Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2022-09-24  5:46         ` Eli Zaretskii
  0 siblings, 2 replies; 15+ messages in thread
From: Manuel Giraud @ 2022-09-23 20:43 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 58028

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Manuel Giraud <manuel@ledu-giraud.fr>
>> Cc: 58028@debbugs.gnu.org
>> Date: Fri, 23 Sep 2022 17:59:48 +0200
>> 
>> Eli Zaretskii <eliz@gnu.org> writes:
>> 
>> >> From: Manuel Giraud <manuel@ledu-giraud.fr>
>> >> Date: Fri, 23 Sep 2022 17:22:07 +0200
>> >> 
>> >> Maybe this problem never triggers.  It is a « better be safe » kind of
>> >> patch.
>> >
>> > Please add an assertion there, to catch the case that previously was
>> > handled by testing only w->desired_matrix.
>> 
>> I do not understand what you mean here.
>
> I mean the assertion that triggers when w->desired_matrix is NULL, but
> w->current_matrix isn't.

Oh I see.  I add the following:
--8<---------------cut here---------------start------------->8---
	  eassert ((w->desired_matrix == NULL) &&
		   (w->current_matrix != NULL));
--8<---------------cut here---------------end--------------->8---

and test a little bit with emacs -Q and nothing break.  So I think
you're right: when w->desired_matrix is NULL, both are.
-- 
Manuel Giraud





^ permalink raw reply	[flat|nested] 15+ messages in thread

* bug#58028: 29.0.50; Ensures no leakage of glyph_matrix
  2022-09-23 20:43       ` Manuel Giraud
@ 2022-09-24  0:26         ` Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2022-09-24  5:46         ` Eli Zaretskii
  1 sibling, 0 replies; 15+ messages in thread
From: Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2022-09-24  0:26 UTC (permalink / raw)
  To: Manuel Giraud; +Cc: 58028, Eli Zaretskii

Manuel Giraud <manuel@ledu-giraud.fr> writes:

> 	  eassert ((w->desired_matrix == NULL) &&
> 		   (w->current_matrix != NULL));

In Emacs, we typically format the code like so:

 	  eassert ((w->desired_matrix == NULL)
 		   && (w->current_matrix != NULL));

> and test a little bit with emacs -Q and nothing break.  So I think
> you're right: when w->desired_matrix is NULL, both are.

BTW, did you build with checking?





^ permalink raw reply	[flat|nested] 15+ messages in thread

* bug#58028: 29.0.50; Ensures no leakage of glyph_matrix
  2022-09-23 20:43       ` Manuel Giraud
  2022-09-24  0:26         ` Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2022-09-24  5:46         ` Eli Zaretskii
  2022-09-24 10:29           ` Manuel Giraud
  1 sibling, 1 reply; 15+ messages in thread
From: Eli Zaretskii @ 2022-09-24  5:46 UTC (permalink / raw)
  To: Manuel Giraud; +Cc: 58028

> From: Manuel Giraud <manuel@ledu-giraud.fr>
> Cc: 58028@debbugs.gnu.org
> Date: Fri, 23 Sep 2022 22:43:53 +0200
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> From: Manuel Giraud <manuel@ledu-giraud.fr>
> >> Cc: 58028@debbugs.gnu.org
> >> Date: Fri, 23 Sep 2022 17:59:48 +0200
> >> 
> >> Eli Zaretskii <eliz@gnu.org> writes:
> >> 
> >> >> From: Manuel Giraud <manuel@ledu-giraud.fr>
> >> >> Date: Fri, 23 Sep 2022 17:22:07 +0200
> >> >> 
> >> >> Maybe this problem never triggers.  It is a « better be safe » kind of
> >> >> patch.
> >> >
> >> > Please add an assertion there, to catch the case that previously was
> >> > handled by testing only w->desired_matrix.
> >> 
> >> I do not understand what you mean here.
> >
> > I mean the assertion that triggers when w->desired_matrix is NULL, but
> > w->current_matrix isn't.
> 
> Oh I see.  I add the following:
> --8<---------------cut here---------------start------------->8---
> 	  eassert ((w->desired_matrix == NULL) &&
> 		   (w->current_matrix != NULL));
> --8<---------------cut here---------------end--------------->8---
> 
> and test a little bit with emacs -Q and nothing break.  So I think
> you're right: when w->desired_matrix is NULL, both are.

Yes, that's what I meant.

So we can install your change with the above assertion added, and then
wait for the assertion to trigger and investigate the case(s) where it
triggers to see whether we are missing something.

Thanks.





^ permalink raw reply	[flat|nested] 15+ messages in thread

* bug#58028: 29.0.50; Ensures no leakage of glyph_matrix
  2022-09-24  5:46         ` Eli Zaretskii
@ 2022-09-24 10:29           ` Manuel Giraud
  2022-09-24 10:34             ` Lars Ingebrigtsen
  2022-09-24 12:31             ` Eli Zaretskii
  0 siblings, 2 replies; 15+ messages in thread
From: Manuel Giraud @ 2022-09-24 10:29 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 58028

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

Eli Zaretskii <eliz@gnu.org> writes:

[...]

> Yes, that's what I meant.
>
> So we can install your change with the above assertion added, and then
> wait for the assertion to trigger and investigate the case(s) where it
> triggers to see whether we are missing something.

Ok.  So, as Po suggested, I did a "make check" with and without the new
attached patch and get the same following summary result:

SUMMARY OF TEST RESULTS
-----------------------
Files examined: 450
Ran 6454 tests, 6340 results as expected, 18 unexpected, 96 skipped
1 files did not contain any tests:
  src/emacs-module-tests.log
10 files contained unexpected results:
  src/process-tests.log
  lisp/wdired-tests.log
  lisp/vc/vc-tests.log
  lisp/play/fortune-tests.log
  lisp/net/nsm-tests.log
  lisp/gnus/gnus-icalendar-tests.log
  lisp/epg-tests.log
  lisp/calendar/icalendar-tests.log
  lisp/bookmark-tests.log
  lisp/net/tramp-tests.log

Thanks.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Ensures-no-leakage-of-glyph_matrix.patch --]
[-- Type: text/x-patch, Size: 1153 bytes --]

From ecd618cded21b2ad9d02c991e8f63b5137fe7b41 Mon Sep 17 00:00:00 2001
From: Manuel Giraud <manuel@ledu-giraud.fr>
Date: Fri, 23 Sep 2022 17:14:44 +0200
Subject: [PATCH] Ensures no leakage of glyph_matrix

; * src/dispnew.c (allocate_matrices_for_window_redisplay): Ensures no
leakage of glyph_matrix.
---
 src/dispnew.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/src/dispnew.c b/src/dispnew.c
index b786f0f1ba..9e4b43087e 100644
--- a/src/dispnew.c
+++ b/src/dispnew.c
@@ -1807,11 +1807,13 @@ allocate_matrices_for_window_redisplay (struct window *w)
 	  struct dim dim;
 
 	  /* If matrices are not yet allocated, allocate them now.  */
+	  eassert ((w->desired_matrix == NULL)
+		   && (w->current_matrix != NULL));
 	  if (w->desired_matrix == NULL)
-	    {
-	      w->desired_matrix = new_glyph_matrix (NULL);
-	      w->current_matrix = new_glyph_matrix (NULL);
-	    }
+	    w->desired_matrix = new_glyph_matrix (NULL);
+
+	  if (w->current_matrix == NULL)
+	    w->current_matrix = new_glyph_matrix (NULL);
 
 	  dim.width = required_matrix_width (w);
 	  dim.height = required_matrix_height (w);
-- 
2.37.3


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

-- 
Manuel Giraud

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* bug#58028: 29.0.50; Ensures no leakage of glyph_matrix
  2022-09-24 10:29           ` Manuel Giraud
@ 2022-09-24 10:34             ` Lars Ingebrigtsen
  2022-09-24 10:36               ` Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2022-09-24 12:31             ` Eli Zaretskii
  1 sibling, 1 reply; 15+ messages in thread
From: Lars Ingebrigtsen @ 2022-09-24 10:34 UTC (permalink / raw)
  To: Manuel Giraud; +Cc: 58028, Eli Zaretskii

Manuel Giraud <manuel@ledu-giraud.fr> writes:

> Ok.  So, as Po suggested, I did a "make check" with and without the new
> attached patch and get the same following summary result:

I think Po Lu meant building Emacs with checking, otherwise those
asserts won't be compiled in.  I.e., 

./configure --enable-checking=all; make






^ permalink raw reply	[flat|nested] 15+ messages in thread

* bug#58028: 29.0.50; Ensures no leakage of glyph_matrix
  2022-09-24 10:34             ` Lars Ingebrigtsen
@ 2022-09-24 10:36               ` Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors
  0 siblings, 0 replies; 15+ messages in thread
From: Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2022-09-24 10:36 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: 58028, Eli Zaretskii, Manuel Giraud

Lars Ingebrigtsen <larsi@gnus.org> writes:

> I think Po Lu meant building Emacs with checking, otherwise those
> asserts won't be compiled in.  I.e., 
>
> ./configure --enable-checking=all; make

Indeed, that's what I meant.





^ permalink raw reply	[flat|nested] 15+ messages in thread

* bug#58028: 29.0.50; Ensures no leakage of glyph_matrix
  2022-09-24 10:29           ` Manuel Giraud
  2022-09-24 10:34             ` Lars Ingebrigtsen
@ 2022-09-24 12:31             ` Eli Zaretskii
  2022-09-24 15:31               ` Manuel Giraud
  1 sibling, 1 reply; 15+ messages in thread
From: Eli Zaretskii @ 2022-09-24 12:31 UTC (permalink / raw)
  To: Manuel Giraud; +Cc: 58028

> From: Manuel Giraud <manuel@ledu-giraud.fr>
> Cc: 58028@debbugs.gnu.org
> Date: Sat, 24 Sep 2022 12:29:17 +0200
> 
> +	  eassert ((w->desired_matrix == NULL)
> +		   && (w->current_matrix != NULL));
>  	  if (w->desired_matrix == NULL)
> -	    {
> -	      w->desired_matrix = new_glyph_matrix (NULL);
> -	      w->current_matrix = new_glyph_matrix (NULL);
> -	    }
> +	    w->desired_matrix = new_glyph_matrix (NULL);
> +
> +	  if (w->current_matrix == NULL)
> +	    w->current_matrix = new_glyph_matrix (NULL);

I think this should be

	  /* If matrices are not yet allocated, allocate them now.  */
	  if (w->desired_matrix == NULL)
	    {
	      w->desired_matrix = new_glyph_matrix (NULL);
	      eassert (w->current_matrix == NULL);
	    }

	  if (w->current_matrix == NULL)
	    w->current_matrix = new_glyph_matrix (NULL);





^ permalink raw reply	[flat|nested] 15+ messages in thread

* bug#58028: 29.0.50; Ensures no leakage of glyph_matrix
  2022-09-24 12:31             ` Eli Zaretskii
@ 2022-09-24 15:31               ` Manuel Giraud
  2022-09-24 16:09                 ` Eli Zaretskii
  0 siblings, 1 reply; 15+ messages in thread
From: Manuel Giraud @ 2022-09-24 15:31 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 58028

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

So I've tested emacs -Q a bit (compiled with --enable-checking=all) with
the attached patch.  I had no error so far.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Ensures-no-leakage-of-glyph_matrix.patch --]
[-- Type: text/x-patch, Size: 1031 bytes --]

From 332ea1a172568a31f8e2ad0dcd921727d30dad68 Mon Sep 17 00:00:00 2001
From: Manuel Giraud <manuel@ledu-giraud.fr>
Date: Fri, 23 Sep 2022 17:14:44 +0200
Subject: [PATCH] Ensures no leakage of glyph_matrix

; * src/dispnew.c (allocate_matrices_for_window_redisplay): Ensures no
leakage of glyph_matrix.
---
 src/dispnew.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/src/dispnew.c b/src/dispnew.c
index b786f0f1ba..2568ba1086 100644
--- a/src/dispnew.c
+++ b/src/dispnew.c
@@ -1810,9 +1810,12 @@ allocate_matrices_for_window_redisplay (struct window *w)
 	  if (w->desired_matrix == NULL)
 	    {
 	      w->desired_matrix = new_glyph_matrix (NULL);
-	      w->current_matrix = new_glyph_matrix (NULL);
+	      eassert (w->current_matrix == NULL);
 	    }
 
+	  if (w->current_matrix == NULL)
+	    w->current_matrix = new_glyph_matrix (NULL);
+
 	  dim.width = required_matrix_width (w);
 	  dim.height = required_matrix_height (w);
 	  adjust_glyph_matrix (w, w->desired_matrix, 0, 0, dim);
-- 
2.37.3


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

-- 
Manuel Giraud

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* bug#58028: 29.0.50; Ensures no leakage of glyph_matrix
  2022-09-24 15:31               ` Manuel Giraud
@ 2022-09-24 16:09                 ` Eli Zaretskii
  2022-09-25 16:51                   ` Manuel Giraud
  0 siblings, 1 reply; 15+ messages in thread
From: Eli Zaretskii @ 2022-09-24 16:09 UTC (permalink / raw)
  To: Manuel Giraud; +Cc: 58028-done

> From: Manuel Giraud <manuel@ledu-giraud.fr>
> Cc: 58028@debbugs.gnu.org
> Date: Sat, 24 Sep 2022 17:31:20 +0200
> 
> So I've tested emacs -Q a bit (compiled with --enable-checking=all) with
> the attached patch.  I had no error so far.

Thanks, installed.

Please in the future mention the bug number in the commit log message
(I added it for you this time).





^ permalink raw reply	[flat|nested] 15+ messages in thread

* bug#58028: 29.0.50; Ensures no leakage of glyph_matrix
  2022-09-24 16:09                 ` Eli Zaretskii
@ 2022-09-25 16:51                   ` Manuel Giraud
  2022-09-25 19:14                     ` Eli Zaretskii
  0 siblings, 1 reply; 15+ messages in thread
From: Manuel Giraud @ 2022-09-25 16:51 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 58028-done

Eli Zaretskii <eliz@gnu.org> writes:

> Thanks, installed.

Thanks.  But I wonder what is the point of this eassert if it have to be
activated at compile time?

> Please in the future mention the bug number in the commit log message
> (I added it for you this time).

Ok, I'll try to remember this next time.  BTW, how could you do it
*before* submitting a bug report?

Best regards,
-- 
Manuel Giraud





^ permalink raw reply	[flat|nested] 15+ messages in thread

* bug#58028: 29.0.50; Ensures no leakage of glyph_matrix
  2022-09-25 16:51                   ` Manuel Giraud
@ 2022-09-25 19:14                     ` Eli Zaretskii
  0 siblings, 0 replies; 15+ messages in thread
From: Eli Zaretskii @ 2022-09-25 19:14 UTC (permalink / raw)
  To: Manuel Giraud; +Cc: 58028-done

> From: Manuel Giraud <manuel@ledu-giraud.fr>
> Cc: 58028-done@debbugs.gnu.org
> Date: Sun, 25 Sep 2022 18:51:38 +0200
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > Thanks, installed.
> 
> Thanks.  But I wonder what is the point of this eassert if it have to be
> activated at compile time?

We believe that the assertion will never trigger, so having it in a
production build is a net loss: users could lose their edits.

OTOH, some of the developers (myself included) routinely compile Emacs
with --enable-checking, which activates the assertions.  So if this
assertion is ever contradicted, chances are that one of the developers
will bump into that situation, and will report it (or fix it).

> > Please in the future mention the bug number in the commit log message
> > (I added it for you this time).
> 
> Ok, I'll try to remember this next time.  BTW, how could you do it
> *before* submitting a bug report?

You don't.  But once the bug report has more than a single message,
the number is known, so patches submitted after that should mention
the bug number.





^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2022-09-25 19:14 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-23 15:22 bug#58028: 29.0.50; Ensures no leakage of glyph_matrix Manuel Giraud
2022-09-23 15:49 ` Eli Zaretskii
2022-09-23 15:59   ` Manuel Giraud
2022-09-23 17:43     ` Eli Zaretskii
2022-09-23 20:43       ` Manuel Giraud
2022-09-24  0:26         ` Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors
2022-09-24  5:46         ` Eli Zaretskii
2022-09-24 10:29           ` Manuel Giraud
2022-09-24 10:34             ` Lars Ingebrigtsen
2022-09-24 10:36               ` Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors
2022-09-24 12:31             ` Eli Zaretskii
2022-09-24 15:31               ` Manuel Giraud
2022-09-24 16:09                 ` Eli Zaretskii
2022-09-25 16:51                   ` Manuel Giraud
2022-09-25 19:14                     ` Eli Zaretskii

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).