On Friday 27 June 2008 06:22:02 [EMAIL PROTECTED] wrote:
> Modified:
> trunk/src/headers.c
>
> Log:
> [core] update function-level documentation in src/headers.c, add a few XXX
> notes concerning questions and suggestions I have.
>
> Modified: trunk/src/headers.c
> --- trunk/src/headers.c (original)
> +++ trunk/src/headers.c Fri Jun 27 06:22:01 2008
> @@ -246,6 +255,10 @@
> /* Expand the array of sized resource pools, if necessary */
> if (num_old <= idx) {
> const UINTVAL num_new = idx + 1;
> + /* XXX: use mem_sys_realloc_zeroed to do this easier? If we want the
> + same debugging behavior as mem_internal_realloc, we would
> + need to add a new function/macro for
> + mem_internal_realloc_zeroed, to mirror
> mem_sys_realloc_zeroed. */
Agreed.
> @@ -341,6 +361,8 @@
> new_pmc_ext(PARROT_INTERP)
> {
> Small_Object_Pool * const pool = interp->arena_base->pmc_ext_pool;
> + /* XXX: Should we check here to ensure the PMC_EXT is non-null
> + like we do in C<new_pmc>? */
> return (PMC_EXT *)pool->get_free_object(interp, pool);
> }
Yes.
> @@ -387,6 +413,8 @@
> if (!PObj_is_PMC_EXT_TEST(pmc)) {
> add_pmc_ext(interp, pmc);
> }
> + /* XXX: Should we test the Sync* for non-null? should we allocate
> these + from a bufferlike pool instead of directly from the
> system? */ PMC_sync(pmc) = (Sync *)mem_internal_allocate(sizeof
> (*PMC_sync(pmc))); PMC_sync(pmc)->owner = interp;
> MUTEX_INIT(PMC_sync(pmc)->pmc_lock);
Yes.
> @@ -732,7 +797,9 @@
>
> =item C<static void free_pool>
>
> -Loops backwards through the provided pool, freeing all of its arenas.
> +Frees a pool and all of it's arenas. Loops through the list of arenas
> +backwards and returns each to the memory manager. Then, it frees the
> +pool structure itself.
>
> =cut
>
s/it's/its/
> @@ -756,7 +823,9 @@
>
> =item C<static int sweep_cb_buf>
>
> -Sweeps and frees the provided pool. Returns 0.
> +Performs a final garbage collection sweep, and then frees the pool. Calls
> +C<Parrot_dod_sweep> to perform the sweep, and C<free_pool> to free the
> +pool and all it's arenas.
>
> =cut
s/it's/its/
> @@ -877,6 +951,8 @@
> PMC_sync(p)->owner = dest_interp;
> }
> else {
> + /* XXX: This error-handling is bad, we should do
> + something more standard here instead. */
> /* fprintf(stderr, "BAD PMC: address=%p,
> base_type=%d\n",
> p, p->vtable->base_type); */
I'm not sure what else to do than throw an exception. PARROT_ASSERT might be
useful.
-- c