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

Reply via email to