From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: ludovic.courtes@laas.fr (Ludovic =?iso-8859-1?Q?Court=E8s?=) Newsgroups: gmane.lisp.guile.devel Subject: Re: [PATCH] Reference leak in `iprin1 ()' Date: Mon, 14 Nov 2005 10:58:46 +0100 Organization: LAAS-CNRS Message-ID: <874q6fsg89.fsf@laas.fr> References: <87y83z3vh5.fsf@laas.fr> <4371CF46.4010708@xs4all.nl> <87y83xkcq6.fsf@laas.fr> <8764r18r2j.fsf@zip.com.au> <87wtjghm1m.fsf_-_@laas.fr> <87zmoa2pad.fsf@ossau.uklinux.net> NNTP-Posting-Host: main.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Transfer-Encoding: quoted-printable X-Trace: sea.gmane.org 1131970475 5922 80.91.229.2 (14 Nov 2005 12:14:35 GMT) X-Complaints-To: usenet@sea.gmane.org NNTP-Posting-Date: Mon, 14 Nov 2005 12:14:35 +0000 (UTC) Cc: Han-Wen Nienhuys , guile-devel@gnu.org Original-X-From: guile-devel-bounces+guile-devel=m.gmane.org@gnu.org Mon Nov 14 13:14:33 2005 Return-path: Original-Received: from lists.gnu.org ([199.232.76.165]) by ciao.gmane.org with esmtp (Exim 4.43) id 1EbdBV-0003TH-PW for guile-devel@m.gmane.org; Mon, 14 Nov 2005 13:11:42 +0100 Original-Received: from localhost ([127.0.0.1] helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1EbdBT-0005F7-0y for guile-devel@m.gmane.org; Mon, 14 Nov 2005 07:11:39 -0500 Original-Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1Ebb72-0000xV-Nx for guile-devel@gnu.org; Mon, 14 Nov 2005 04:58:57 -0500 Original-Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1Ebb70-0000xA-Tf for guile-devel@gnu.org; Mon, 14 Nov 2005 04:58:55 -0500 Original-Received: from [199.232.76.173] (helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Ebb6y-0000x5-M1 for guile-devel@gnu.org; Mon, 14 Nov 2005 04:58:53 -0500 Original-Received: from [140.93.0.15] (helo=laas.laas.fr) by monty-python.gnu.org with esmtp (TLS-1.0:DHE_RSA_3DES_EDE_CBC_SHA:24) (Exim 4.34) id 1Ebb6y-0002Uq-Nl for guile-devel@gnu.org; Mon, 14 Nov 2005 04:58:53 -0500 Original-Received: by laas.laas.fr (8.13.1/8.13.4) with SMTP id jAE9wlRq003594; Mon, 14 Nov 2005 10:58:49 +0100 (CET) Original-To: Neil Jerram X-URL: http://www.laas.fr/~lcourtes/ X-Revolutionary-Date: 24 Brumaire an 214 de la =?iso-8859-1?Q?R=E9volution?= X-PGP-Key-ID: 0xEB1F5364 X-PGP-Key: http://www.laas.fr/~lcourtes/ludovic.asc X-PGP-Fingerprint: 821D 815D 902A 7EAB 5CEE D120 7FBA 3D4F EB1F 5364 X-OS: powerpc-unknown-linux-gnu Mail-Followup-To: Neil Jerram , guile-devel@gnu.org, Han-Wen Nienhuys In-Reply-To: <87zmoa2pad.fsf@ossau.uklinux.net> (Neil Jerram's message of "Sat, 12 Nov 2005 09:23:06 +0000") User-Agent: Gnus/5.110004 (No Gnus v0.4) Emacs/21.4 (gnu/linux) X-Spam-Score: 0 () X-Scanned-By: MIMEDefang at CNRS-LAAS X-MIME-Autoconverted: from 8bit to quoted-printable by laas.laas.fr id jAE9wlRq003594 X-BeenThere: guile-devel@gnu.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "Developers list for Guile, the GNU extensibility library" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Original-Sender: guile-devel-bounces+guile-devel=m.gmane.org@gnu.org Errors-To: guile-devel-bounces+guile-devel=m.gmane.org@gnu.org Xref: news.gmane.org gmane.lisp.guile.devel:5392 Archived-At: Hello, Neil Jerram writes: > Nice work, but it looks to me that PUSH_REF sets the value of the > (pstate->top)th element _before_ incrementing pstate->top. So > shouldn't your fix do the decrement first and then set the slot to > undefined? Yes, you're perfectly right. > (Note that your fix will still prevent most reference leaks, just not > the outermost one. So that's why it may appear to be working > already.) Exactly. > Also, is the () -> (void) change strictly related? In fact, it's strictly unrelated, but while we're at it (really `(void)' was meant here, not `()')... Below is an updated patch. I modified `PUSH_REF ()' as well so that it does PSTATE->TOP++ _after_ using the `PSTATE_STACK_SET ()' macro: this is safer. And, well, I couldn't resist the desire to "beautifully backslashify" `ENTER_NESTED_DATA ()' as well. :-) I hope this is not too much pollution for you. Thanks, Ludovic. 2005-11-14 Ludovic Court=E8s * print.c (EXIT_NESTED_DATA): Before popping from the stack, reset the value at its top. This fixes a reference leak. (PUSH_REF): Perform `pstate->top++' after calling `PSTATE_STACK_SET ()' in order to avoid undesired side effects. (scm_current_pstate): Replaced `()' by `(void)'. --- orig/libguile/print.c +++ mod/libguile/print.c @@ -112,31 +112,40 @@ * time complexity (O (depth * N)), The printer code can be * rewritten to be O(N). */ -#define PUSH_REF(pstate, obj) \ -do { \ - PSTATE_STACK_SET (pstate, pstate->top++, obj); \ - if (pstate->top =3D=3D pstate->ceiling) \ - grow_ref_stack (pstate); \ +#define PUSH_REF(pstate, obj) \ +do \ +{ \ + PSTATE_STACK_SET (pstate, pstate->top, obj); \ + pstate->top++; \ + if (pstate->top =3D=3D pstate->ceiling) \ + grow_ref_stack (pstate); \ } while(0) =20 -#define ENTER_NESTED_DATA(pstate, obj, label) \ -do { \ - register unsigned long i; \ - for (i =3D 0; i < pstate->top; ++i) \ - if (scm_is_eq (PSTATE_STACK_REF (pstate, i), (obj))) \ - goto label; \ - if (pstate->fancyp) \ - { \ - if (pstate->top - pstate->list_offset >=3D pstate->level) \ - { \ - scm_putc ('#', port); \ - return; \ - } \ - } \ - PUSH_REF(pstate, obj); \ +#define ENTER_NESTED_DATA(pstate, obj, label) \ +do \ +{ \ + register unsigned long i; \ + for (i =3D 0; i < pstate->top; ++i) \ + if (scm_is_eq (PSTATE_STACK_REF (pstate, i), (obj))) \ + goto label; \ + if (pstate->fancyp) \ + { \ + if (pstate->top - pstate->list_offset >=3D pstate->level) \ + { \ + scm_putc ('#', port); \ + return; \ + } \ + } \ + PUSH_REF(pstate, obj); \ } while(0) =20 -#define EXIT_NESTED_DATA(pstate) { --pstate->top; } +#define EXIT_NESTED_DATA(pstate) \ +do \ +{ \ + --pstate->top; \ + PSTATE_STACK_SET (pstate, pstate->top, SCM_UNDEFINED); \ +} \ +while (0) =20 SCM scm_print_state_vtable =3D SCM_BOOL_F; static SCM print_state_pool =3D SCM_EOL; @@ -144,8 +153,8 @@ =20 #ifdef GUILE_DEBUG /* Used for debugging purposes */ =20 -SCM_DEFINE (scm_current_pstate, "current-pstate", 0, 0, 0,=20 - (), +SCM_DEFINE (scm_current_pstate, "current-pstate", 0, 0, 0, + (void), "Return the current-pstate -- the car of the\n" "@code{print_state_pool}. @code{current-pstate} is only\n" "included in @code{--enable-guile-debug} builds.") _______________________________________________ Guile-devel mailing list Guile-devel@gnu.org http://lists.gnu.org/mailman/listinfo/guile-devel