* [PATCH] some misuse of GCPROs @ 2011-11-18 14:06 Dmitry Antipov 2011-11-18 16:51 ` Stefan Monnier 0 siblings, 1 reply; 5+ messages in thread From: Dmitry Antipov @ 2011-11-18 14:06 UTC (permalink / raw) To: emacs-devel [-- Attachment #1: Type: text/plain, Size: 70 bytes --] ...found while hacking around GC_MARK_STACK with DEBUG_GCPRO. Dmitry [-- Attachment #2: gcpro_fixes.patch --] [-- Type: text/plain, Size: 1992 bytes --] === modified file 'src/ChangeLog' --- src/ChangeLog 2011-11-18 12:41:36 +0000 +++ src/ChangeLog 2011-11-18 14:02:15 +0000 @@ -1,3 +1,9 @@ +2011-11-18 Dmitry Antipov <dmantipov@yandex.ru> + + * keymap.c (Fwhere_is_internal): Add missing RETURN_UNGCPROs. + + * fileio.c (Finsert_file_contents): Use inner_gcpro. + 2011-11-18 Eli Zaretskii <eliz@gnu.org> * dispnew.c (swap_glyph_pointers): Swap the used[] arrays and the === modified file 'src/fileio.c' --- src/fileio.c 2011-09-30 20:22:01 +0000 +++ src/fileio.c 2011-11-18 13:58:04 +0000 @@ -3686,6 +3686,7 @@ int this_count = SPECPDL_INDEX (); int multibyte = ! NILP (BVAR (current_buffer, enable_multibyte_characters)); Lisp_Object conversion_buffer; + struct gcpro inner_gcpro1; conversion_buffer = code_conversion_save (1, multibyte); @@ -3701,7 +3702,7 @@ inserted = 0; /* Bytes put into CONVERSION_BUFFER so far. */ unprocessed = 0; /* Bytes not processed in previous loop. */ - GCPRO1 (conversion_buffer); + GCPRO1_VAR (conversion_buffer, inner_gcpro); while (how_much < total) { /* We read one bunch by one (READ_BUF_SIZE bytes) to allow @@ -3729,7 +3730,7 @@ if (coding.carryover_bytes > 0) memcpy (read_buf, coding.carryover, unprocessed); } - UNGCPRO; + UNGCPRO_VAR (inner_gcpro); emacs_close (fd); /* We should remove the unwind_protect calling === modified file 'src/keymap.c' --- src/keymap.c 2011-11-17 17:40:48 +0000 +++ src/keymap.c 2011-11-18 13:50:01 +0000 @@ -2624,11 +2624,11 @@ /* We have a list of advertised bindings. */ while (CONSP (tem)) if (EQ (shadow_lookup (keymaps, XCAR (tem), Qnil, 0), definition)) - return XCAR (tem); + RETURN_UNGCPRO (XCAR (tem)); else tem = XCDR (tem); if (EQ (shadow_lookup (keymaps, tem, Qnil, 0), definition)) - return tem; + RETURN_UNGCPRO (tem); } sequences = Freverse (where_is_internal (definition, keymaps, ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] some misuse of GCPROs 2011-11-18 14:06 [PATCH] some misuse of GCPROs Dmitry Antipov @ 2011-11-18 16:51 ` Stefan Monnier 2011-11-18 17:24 ` Dmitry Antipov 2011-11-18 19:04 ` Paul Eggert 0 siblings, 2 replies; 5+ messages in thread From: Stefan Monnier @ 2011-11-18 16:51 UTC (permalink / raw) To: Dmitry Antipov; +Cc: emacs-devel > + * keymap.c (Fwhere_is_internal): Add missing RETURN_UNGCPROs. Thanks, installed. > + * fileio.c (Finsert_file_contents): Use inner_gcpro. I really dislike this inner_gcpro business, so I haven't installed this hunk. What is it needed for? Stefan ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] some misuse of GCPROs 2011-11-18 16:51 ` Stefan Monnier @ 2011-11-18 17:24 ` Dmitry Antipov 2011-11-20 4:35 ` Stefan Monnier 2011-11-18 19:04 ` Paul Eggert 1 sibling, 1 reply; 5+ messages in thread From: Dmitry Antipov @ 2011-11-18 17:24 UTC (permalink / raw) To: Stefan Monnier; +Cc: emacs-devel On 11/18/2011 08:51 PM, Stefan Monnier wrote: > I really dislike this inner_gcpro business, so I haven't installed > this hunk. What is it needed for? I too, but this is a fix for 'reuse GCPRO' error. The following code ... Lisp_Object foo; struct gcpro gcpro1; ... GCPRO1 (foo); ... if (...) { Lisp_Object bar; ... GCPRO1 (bar); ... UNGCPRO; } .... is wrong because it clears the protection of 'foo' by re-using 'gcpro1' to protect 'bar'. It should be: ... Lisp_Object foo; struct gcpro gcpro1; ... GCPRO1 (foo); ... if (...) { Lisp_Object bar; struct gcpro inner_gcpro1; ... GCPRO1_VAR (bar, inner_gcpro); ... UNGCPRO_VAR (inner_gcpro); } .... Dmitry ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] some misuse of GCPROs 2011-11-18 17:24 ` Dmitry Antipov @ 2011-11-20 4:35 ` Stefan Monnier 0 siblings, 0 replies; 5+ messages in thread From: Stefan Monnier @ 2011-11-20 4:35 UTC (permalink / raw) To: Dmitry Antipov; +Cc: emacs-devel >> I really dislike this inner_gcpro business, so I haven't installed >> this hunk. What is it needed for? > I too, but this is a fix for 'reuse GCPRO' error. The following code Oh, I see, thanks. I added the patch below, then. Stefan === modified file 'src/fileio.c' --- src/fileio.c 2011-11-20 02:29:42 +0000 +++ src/fileio.c 2011-11-20 04:31:52 +0000 @@ -3686,6 +3686,7 @@ int this_count = SPECPDL_INDEX (); int multibyte = ! NILP (BVAR (current_buffer, enable_multibyte_characters)); Lisp_Object conversion_buffer; + struct gcpro gcpro1; conversion_buffer = code_conversion_save (1, multibyte); ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] some misuse of GCPROs 2011-11-18 16:51 ` Stefan Monnier 2011-11-18 17:24 ` Dmitry Antipov @ 2011-11-18 19:04 ` Paul Eggert 1 sibling, 0 replies; 5+ messages in thread From: Paul Eggert @ 2011-11-18 19:04 UTC (permalink / raw) To: Stefan Monnier; +Cc: Dmitry Antipov, emacs-devel On 11/18/11 08:51, Stefan Monnier wrote: > I really dislike this inner_gcpro business, so I haven't installed > this hunk. What is it needed for? It's needed to prevent gcprolist from becoming corrupted when GC_MARK_STACK is not GC_MAKE_GCPROS_NOOPS. Without the patch, Finsert_file_contents sometimes invokes GCPRO5 and then GCPRO1 on the same struct gcpro, without an intervening UNGCPRO. This corrupts the list. Here's a proposed alternative patch that avoids inner_gcpro. It's simpler code, though it has the minor run-time disadvantage of GCPROing conversion_buffer more than is strictly needed (until insert-file-contents returns). === modified file 'src/ChangeLog' --- src/ChangeLog 2011-11-18 18:29:29 +0000 +++ src/ChangeLog 2011-11-18 18:59:32 +0000 @@ -1,5 +1,9 @@ 2011-11-18 Paul Eggert <eggert@cs.ucla.edu> + * fileio.c (Finsert_file_contents): Don't corrupt gcprolist + if GC_MARK_STACK is not GC_MAKE_GCPROS_NOOPS. Reported by Dmitry Antipov + in <http://lists.gnu.org/archive/html/emacs-devel/2011-11/msg00263.html>. + Fix minor problems found by static checking. * dispextern.h, xdisp.c (row_hash): Declare extern only if XASSERTS. * dispnew.c (verify_row_hash): Now static. === modified file 'src/fileio.c' --- src/fileio.c 2011-09-30 20:22:01 +0000 +++ src/fileio.c 2011-11-18 19:00:20 +0000 @@ -3182,8 +3182,9 @@ off_t beg_offset, end_offset; register EMACS_INT unprocessed; int count = SPECPDL_INDEX (); - struct gcpro gcpro1, gcpro2, gcpro3, gcpro4, gcpro5; + struct gcpro gcpro1, gcpro2, gcpro3, gcpro4, gcpro5, gcpro6; Lisp_Object handler, val, insval, orig_filename, old_undo; + Lisp_Object conversion_buffer; Lisp_Object p; EMACS_INT total = 0; int not_regular = 0; @@ -3209,8 +3210,9 @@ p = Qnil; orig_filename = Qnil; old_undo = Qnil; + conversion_buffer = Qnil; - GCPRO5 (filename, val, p, orig_filename, old_undo); + GCPRO6 (filename, val, p, orig_filename, old_undo, conversion_buffer); CHECK_STRING (filename); filename = Fexpand_file_name (filename, Qnil); @@ -3685,7 +3687,6 @@ EMACS_INT this = 0; int this_count = SPECPDL_INDEX (); int multibyte = ! NILP (BVAR (current_buffer, enable_multibyte_characters)); - Lisp_Object conversion_buffer; conversion_buffer = code_conversion_save (1, multibyte); @@ -3701,7 +3702,6 @@ inserted = 0; /* Bytes put into CONVERSION_BUFFER so far. */ unprocessed = 0; /* Bytes not processed in previous loop. */ - GCPRO1 (conversion_buffer); while (how_much < total) { /* We read one bunch by one (READ_BUF_SIZE bytes) to allow @@ -3729,7 +3729,6 @@ if (coding.carryover_bytes > 0) memcpy (read_buf, coding.carryover, unprocessed); } - UNGCPRO; emacs_close (fd); /* We should remove the unwind_protect calling ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2011-11-20 4:35 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2011-11-18 14:06 [PATCH] some misuse of GCPROs Dmitry Antipov 2011-11-18 16:51 ` Stefan Monnier 2011-11-18 17:24 ` Dmitry Antipov 2011-11-20 4:35 ` Stefan Monnier 2011-11-18 19:04 ` Paul Eggert
Code repositories for project(s) associated with this external index https://git.savannah.gnu.org/cgit/emacs.git https://git.savannah.gnu.org/cgit/emacs/org-mode.git This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.