Maksym Planeta, le Sat 07 Apr 2012 21:42:04 +0300, a écrit :
> Samuel Thibault <samuel.thiba...@gnu.org> writes:
>
> > Maksym Planeta, le Sat 07 Apr 2012 19:51:56 +0300, a écrit :
> >> Here is initialization code from pager_alloc():
> >> if (INDIRECT_PAGEMAP(size)) {
> >> alloc_size = INDIRECT_PAGEMAP_SIZE(size);
> >> init_value = (dp_map_t)0;
> >>
> >> And from pager_extend():
> >> for (; i < INDIRECT_PAGEMAP_ENTRIES(new_size); i++)
> >> new_mapptr[i].indirect = (dp_map_t)0;
> >>
> >> As you can see instead of NULL, (dp_map_t)0 is used.
> >
> > And can be put into pager->map in pager_truncate, ok. I'm however not
> > sure we really want to put ifs everywhere. The comment in the truncation
> > says
> >
> > /* We are truncating to a size small enough that it goes to using
> > a one-level map. We already have that map, as the first and only
> > nonempty element in our indirect map. */
> >
> > i.e. the code assumes that map[0].indirect is not NULL. I'd say we
> > should rather allocate an empty map in such case, to keep the rest of
> > the code simple.
> >
>
> And what is the alternative for ifs? longjumps and setjumps?
No: as I said, allocate an empty map, so that the existing code can poke
at it without testing for its presence or not.
> Purpose of this conditions is checking whether map (or submap) is
> already empty.
Not empty, but allocated.
> >> > There is also an issue with
> >> >
> >> > + if (!pager->map) {
> >> > + invalidate_block (pager_offset);
> >> > + goto done;
> >> > + }
> >> > pager_offset = pager->map[f_page];
> >> >
> >> > at that point, pager_offset is not initialized yet...
> >> >
> >>
> >> invalidate_block is a macro that sets pager_offset, so, really,
> >> pager_offset shouldn't been initialized yet.
> >
> > It sets the *content* pointed by pager_offset. It does not set the
> > pager_offset pointer.
>
> pager_offset is not a pointer, it is a union.
Oops, indeed, sorry about that. I'm still wondering, however: rather
than a goto, why not just putting pager_offset = pager->map[f_page] in
the else part?
Samuel
