From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: Paul Eggert Newsgroups: gmane.emacs.devel Subject: Re: [PATCH] some misuse of GCPROs Date: Fri, 18 Nov 2011 11:04:43 -0800 Organization: UCLA Computer Science Department Message-ID: <4EC6AC4B.2030500@cs.ucla.edu> References: <4EC6667E.3040008@yandex.ru> NNTP-Posting-Host: lo.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit X-Trace: dough.gmane.org 1321643110 16691 80.91.229.12 (18 Nov 2011 19:05:10 GMT) X-Complaints-To: usenet@dough.gmane.org NNTP-Posting-Date: Fri, 18 Nov 2011 19:05:10 +0000 (UTC) Cc: Dmitry Antipov , emacs-devel@gnu.org To: Stefan Monnier Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Fri Nov 18 20:05:06 2011 Return-path: Envelope-to: ged-emacs-devel@m.gmane.org Original-Received: from lists.gnu.org ([140.186.70.17]) by lo.gmane.org with esmtp (Exim 4.69) (envelope-from ) id 1RRTkG-00086E-8T for ged-emacs-devel@m.gmane.org; Fri, 18 Nov 2011 20:05:04 +0100 Original-Received: from localhost ([::1]:55447 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RRTkF-0007Tn-HF for ged-emacs-devel@m.gmane.org; Fri, 18 Nov 2011 14:05:03 -0500 Original-Received: from eggs.gnu.org ([140.186.70.92]:45472) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RRTk9-0007PM-UX for emacs-devel@gnu.org; Fri, 18 Nov 2011 14:05:02 -0500 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1RRTk5-0005Ip-Kx for emacs-devel@gnu.org; Fri, 18 Nov 2011 14:04:57 -0500 Original-Received: from smtp.cs.ucla.edu ([131.179.128.62]:60504) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RRTk5-0005IZ-5r for emacs-devel@gnu.org; Fri, 18 Nov 2011 14:04:53 -0500 Original-Received: from localhost (localhost.localdomain [127.0.0.1]) by smtp.cs.ucla.edu (Postfix) with ESMTP id DA005A60008; Fri, 18 Nov 2011 11:04:49 -0800 (PST) X-Virus-Scanned: amavisd-new at smtp.cs.ucla.edu Original-Received: from smtp.cs.ucla.edu ([127.0.0.1]) by localhost (smtp.cs.ucla.edu [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id UeX0jAWaax1t; Fri, 18 Nov 2011 11:04:45 -0800 (PST) Original-Received: from penguin.cs.ucla.edu (Penguin.CS.UCLA.EDU [131.179.64.200]) by smtp.cs.ucla.edu (Postfix) with ESMTPSA id 57A46A60009; Fri, 18 Nov 2011 11:04:44 -0800 (PST) User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:7.0) Gecko/20110927 Thunderbird/7.0 In-Reply-To: X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6 (newer, 3) X-Received-From: 131.179.128.62 X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Original-Sender: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Xref: news.gmane.org gmane.emacs.devel:146090 Archived-At: 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 + * fileio.c (Finsert_file_contents): Don't corrupt gcprolist + if GC_MARK_STACK is not GC_MAKE_GCPROS_NOOPS. Reported by Dmitry Antipov + in . + 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