Subject: Re: UFS and byteswapping.
To: Pawel Jakub Dawidek <pjd@FreeBSD.org>
From: Manuel Bouyer <bouyer@antioche.eu.org>
List: tech-kern
Date: 02/06/2007 21:49:25
sorry for the delay replying to this,

On Fri, Jan 26, 2007 at 04:33:27PM +0100, Pawel Jakub Dawidek wrote:
> Hi.
> 
> I'm looking at NetBSD changes to make UFS usable on architectures with
> different endianess than the one on which file system was created.
> 
> I found one bug I think and I'm looking for confirmation.
> In ufs_dirremove() function one may find:
> 
> #ifdef UFS_DIRHASH
> 	/*
> 	 * Remove the dirhash entry. This is complicated by the fact
> 	 * that `ep' is the previous entry when dp->i_count != 0.
> 	 */
> 	if (dp->i_dirhash != NULL)
> 		ufsdirhash_remove(dp, (dp->i_count == 0) ? ep :
> 		   (struct direct *)((char *)ep + ep->d_reclen), dp->i_offset);
> #endif
> 
> If my understanding is correct, ep->d_reclen should be replaced with
> ufs_rw16(ep->d_reclen).

Yes, you're right

> 
> The one below is not really an issue, but... Code from ufs_direnter():
> 
> 	dsize = ufs_rw32(ep->d_ino, needswap) ?
> 	    DIRSIZ(FSFMT(dvp), ep, needswap) : 0;
> 
> I've seen many places where you assume that bswap(0) is 0, so I think
> ufs_rw32() can be eliminated:
> 
> 	dsize = ep->d_ino ? DIRSIZ(FSFMT(dvp), ep, needswap) : 0;

In such a case I'd prefer to see explicitely that it's compared against 0:
	dsize = (ep->d_ino != 0) ? DIRSIZ(FSFMT(dvp), ep, needswap) : 0;

this is what I commited
thanks !

-- 
Manuel Bouyer <bouyer@antioche.eu.org>
     NetBSD: 26 ans d'experience feront toujours la difference
--