--Kj7319i9nmIyA2yE
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

On Wed, Oct 24, 2001 at 05:12:26PM -0400, der Mouse wrote:
> Now, the files I'm working with are
> /*	$NetBSD: setup.c,v 1.37 1999/11/15 19:18:26 fvdl Exp $	*/
> /*	$NetBSD: ffs_bswap.c,v 1.7 2000/01/18 18:41:29 bouyer Exp $	*/
> and I wouldn't even bother mentioning this except that it looks to me
> as though they haven't been fully fixed.
> 
> It appears that ffs_sb_swap now tries to work correctly even when both
> arguments are the same (the third argument is now gone; ffs_sb_swap
> deduces it itself).  But three lines from the end, I see it still doing
> ufs_rw32(o->...,needswap), after swapping the field it's trying to
> access.
> 
> Am I hallucinating, or is there still a bug here?  ISTM that
> ufs_sb_swap should be caching pre-swap copies of cpc and nrpos same as
> it does postbloff and postblfmt.

Ahh, good point. This might be an issue, although I didn't
specifically notice a problem in the various fsck_ffs endian
tests I did (which doesn't meant that there *isn't* a problem.)

If you updated to a -current-ish fsck_ffs & ffs_bswap.c, would you
be able to test fsck_ffs on the filesystem that caused the coredump,
and then test the following patch and see if that improves things?

Thanks,
Luke.

--Kj7319i9nmIyA2yE
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename="ffsbswap.dif"

Index: ffs_bswap.c
===================================================================
RCS file: /cvsroot/syssrc/sys/ufs/ffs/ffs_bswap.c,v
retrieving revision 1.13
diff -p -u -r1.13 ffs_bswap.c
--- ffs_bswap.c	2001/09/06 02:16:02	1.13
+++ ffs_bswap.c	2001/10/25 02:42:58
@@ -55,7 +55,7 @@ ffs_sb_swap(struct fs *o, struct fs *n)
 	int i, needswap, len;
 	u_int32_t *o32, *n32;
 	u_int16_t *o16, *n16;
-	u_int32_t postbloff, postblfmt;
+	u_int32_t postbloff, postblfmt, cpc, nrpos;
 
 	if (o->fs_magic == FS_MAGIC) {
 		needswap = 0;
@@ -64,8 +64,11 @@ ffs_sb_swap(struct fs *o, struct fs *n)
 	} else {
 		panic("ffs_sb_swap: can't determine magic");
 	}
+		/* cache values used later, in case o == n */
 	postbloff = ufs_rw32(o->fs_postbloff, needswap);
 	postblfmt = ufs_rw32(o->fs_postblformat, needswap);
+	cpc = ufs_rw32(o->fs_cpc, needswap);
+	nrpos = ufs_rw32(o->fs_nrpos, needswap);
 
 	/*
 	 * In order to avoid a lot of lines, as the first 52 fields of
@@ -97,8 +100,7 @@ ffs_sb_swap(struct fs *o, struct fs *n)
 	n16 = (postblfmt == FS_42POSTBLFMT) ? n->fs_opostbl[0] :
 	    (int16_t *)((u_int8_t *)n + postbloff);
 	len = postblfmt == FS_42POSTBLFMT ?
-	    128 /* fs_opostbl[16][8] */ :
-	    ufs_rw32(o->fs_cpc, needswap) * ufs_rw32(o->fs_nrpos, needswap);
+	    128 /* fs_opostbl[16][8] */ : cpc * nrpos;
 	for (i = 0; i < len; i++)
 		n16[i] = bswap16(o16[i]);
 }

--Kj7319i9nmIyA2yE--