Hello,
Neil Jerram <[EMAIL PROTECTED]> 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 <[EMAIL PROTECTED]>
* 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.")
_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/guile-devel
