Re: Bogus KASSERT() in LFS?

[Eduardo Horvath <eeh%NetBSD.org@localhost>]

On Wed, 5 Jan 2011, Martin Husemann wrote:

> Disclaimer: I know nothing about LFS, but it seems to me that there is no
> guarantee for "curpg" to not be NULL in the following code from
> src/sys/ufs/lfs/lfs_vnops.c:
> 
>         while (by_list || soff < MIN(blkeof, endoffset)) {
>                 if (by_list) {
>                         /*
>                          * Find the first page in a block.  Skip
>                          * blocks outside our area of interest or beyond
>                          * the end of file.
>                          */
>                         KASSERT(curpg == NULL ||
>                             (curpg->flags & PG_MARKER) == 0);
> 
> 
> and actually some ATF tests die for me with SIGSEGV inside the KASSERT.
> So, would this patch be ok?

Interesting......

I think you're right.  While I'm pretty sure that curpg won't be NULL on 
the first iteration, I think it can be NULL on subsequent iterations.  I'd 
commit that change.

Eduardo

> 
> Index: lfs_vnops.c
> ===================================================================
> RCS file: /cvsroot/src/sys/ufs/lfs/lfs_vnops.c,v
> retrieving revision 1.233
> diff -u -r1.233 lfs_vnops.c
> --- lfs_vnops.c       2 Jan 2011 05:09:32 -0000       1.233
> +++ lfs_vnops.c       5 Jan 2011 15:07:00 -0000
> @@ -1860,7 +1860,8 @@
>                        * blocks outside our area of interest or beyond
>                        * the end of file.
>                        */
> -                     KASSERT((curpg->flags & PG_MARKER) == 0);
> +                     KASSERT(curpg == NULL ||
> +                         (curpg->flags & PG_MARKER) == 0);
>                       if (pages_per_block > 1) {
>                               while (curpg &&
>                                   ((curpg->offset & fs->lfs_bmask) ||
>