Hello;
Thanks for reviewing ...
El 01/07/2013 2:25 a. m., Xin Li escribió:
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256
On 6/30/13 8:00 PM, Pedro F. Giffuni wrote:
Author: pfg Date: Mon Jul 1 03:00:15 2013 New Revision: 252435
URL: http://svnweb.freebsd.org/changeset/base/252435
Log: Change i_gen in UFS to an unsigned type.
[...]
Modified: head/sys/ufs/ffs/ffs_vfsops.c
==============================================================================
- --- head/sys/ufs/ffs/ffs_vfsops.c Mon Jul 1 02:48:27 2013
(r252434)
+++ head/sys/ufs/ffs/ffs_vfsops.c Mon Jul 1 03:00:15 2013
(r252435) @@ -1791,7 +1791,7 @@ ffs_vgetf(mp, ino, flags, vpp,
ffs_flags * already have one. This should only happen on old
filesystems. */ if (ip->i_gen == 0) { - ip->i_gen = arc4random() /
2 + 1; + ip->i_gen = arc4random() + 1; if ((vp->v_mount->mnt_flag
& MNT_RDONLY) == 0) { ip->i_flag |= IN_MODIFIED; DIP_SET(ip, i_gen,
ip->i_gen);
Why ip->i_gen must be non-zero here? (I think it does not. Note that
arc4random() can return UINT32_MAX so the new code does not guarantee
this anyway, while old code does).
Zero is the expected value when the disk is very old and has no i_gen.
In reality this is likely to be dead code, the real set up of i_gen
is done in newfs and newfs_random() already produces and unsigned int.
If my understanding of the code is right, I think it doesn't really
hurt to have 0 in ip->i_gen in the places where arc4random() is used
(next time it would be regenerated), so probably we can just use
ip->i_gen = arc4random()?
That is pretty much what newfs_random() does.
However, if I was wrong, you probably want some construction like this:
%%% for (;;) {
%%% ip->i_gen = arc4random();
%%% if (ip->i_gen != UINT32_MAX)
%%% break;
%%% }
%%% ip->i_gen++;
Theorically having an open loop in a filesystem is a bad programming
practice. I think I prefer the simple arc4random but still having
a potential overflow here is not a problem.
Or we probably need to import a variant of arc4random_uniform into kernel?
What we have is sufficient and perhaps somewhat overdesigned, i_gen
doesn't have to be too random at all.
Cheers,
Pedro.
_______________________________________________
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
