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