[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] some misuse of GCPROs
From: |
Paul Eggert |
Subject: |
Re: [PATCH] some misuse of GCPROs |
Date: |
Fri, 18 Nov 2011 11:04:43 -0800 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:7.0) Gecko/20110927 Thunderbird/7.0 |
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 <address@hidden>
+ * 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