[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] Reference leak in `iprin1 ()'
From: |
Ludovic Courtès |
Subject: |
Re: [PATCH] Reference leak in `iprin1 ()' |
Date: |
Mon, 14 Nov 2005 10:58:46 +0100 |
User-agent: |
Gnus/5.110004 (No Gnus v0.4) Emacs/21.4 (gnu/linux) |
Hello,
Neil Jerram <address@hidden> 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ès <address@hidden>
* 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 == pstate->ceiling) \
- grow_ref_stack (pstate); \
+#define PUSH_REF(pstate, obj) \
+do \
+{ \
+ PSTATE_STACK_SET (pstate, pstate->top, obj); \
+ pstate->top++; \
+ if (pstate->top == pstate->ceiling) \
+ grow_ref_stack (pstate); \
} while(0)
-#define ENTER_NESTED_DATA(pstate, obj, label) \
-do { \
- register unsigned long i; \
- for (i = 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 >= pstate->level) \
- { \
- scm_putc ('#', port); \
- return; \
- } \
- } \
- PUSH_REF(pstate, obj); \
+#define ENTER_NESTED_DATA(pstate, obj, label) \
+do \
+{ \
+ register unsigned long i; \
+ for (i = 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 >= pstate->level) \
+ { \
+ scm_putc ('#', port); \
+ return; \
+ } \
+ } \
+ PUSH_REF(pstate, obj); \
} while(0)
-#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)
SCM scm_print_state_vtable = SCM_BOOL_F;
static SCM print_state_pool = SCM_EOL;
@@ -144,8 +153,8 @@
#ifdef GUILE_DEBUG /* Used for debugging purposes */
-SCM_DEFINE (scm_current_pstate, "current-pstate", 0, 0, 0,
- (),
+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.")
- Re: [PATCH] Marking weak alist vectors, (continued)
- Re: [PATCH] Marking weak alist vectors, Marius Vollmer, 2005/11/08
- Re: [PATCH] Marking weak alist vectors, Han-Wen Nienhuys, 2005/11/09
- Re: [PATCH] Marking weak alist vectors, Ludovic Courtès, 2005/11/09
- [PATCH] Reference leak in `iprin1 ()', Ludovic Courtès, 2005/11/10
- Re: [PATCH] Reference leak in `iprin1 ()', Neil Jerram, 2005/11/12
- Re: [PATCH] Reference leak in `iprin1 ()',
Ludovic Courtès <=
- Re: [PATCH] Reference leak in `iprin1 ()', Neil Jerram, 2005/11/16
- Re: [PATCH] Reference leak in `iprin1 ()', Ludovic Courtès, 2005/11/17
- Re: [PATCH] Reference leak in `iprin1 ()', Neil Jerram, 2005/11/17
- Re: [PATCH] Marking weak alist vectors, #2, Ludovic Courtès, 2005/11/23
- Re: [PATCH] Marking weak alist vectors, #2, Han-Wen Nienhuys, 2005/11/23
- Re: [PATCH] Marking weak alist vectors, #2, Ludovic Courtès, 2005/11/24
- Re: [PATCH] Marking weak alist vectors, #2, Kevin Ryde, 2005/11/25
[PATCH] Fixing `gc-live-object-stats', Ludovic Courtès, 2005/11/17