Subject: Re: yamt-vop branch
To: YAMAMOTO Takashi <yamt@mwd.biglobe.ne.jp>
From: Chuck Silvers <chuq@chuq.com>
List: tech-kern
Date: 10/29/2005 10:11:32
On Sun, Oct 30, 2005 at 12:22:32AM +0900, YAMAMOTO Takashi wrote:
> > I looked at the changes, I have just a few comments:
> 
> thanks.
> 
> > cd9660_setattr
> > 	should just return EOPNOTSUPP immediately, the extra checks are silly.
> 
> i thought it's written in the manner to support r/w support in future.
> isn't it the case?

written to support only truncation in the future?  that seems silly too.
if someone ever tries to add any writability to this file system
(which seems very unlikely, since the disk format is not at all conducive
to modification on the fly), they can update this function to do the
right thing for whatever types of modification he adds.


> > ntfs_fsync
> > 	should take a (void *) argument like all the other VOPs.
> 
> i've followed the style in which most of ntfs code is written.
> i'd like to leave it for jdolecek@.

ok.


> > tmpfs_update
> > 	should return void, since it can't fail.
> 
> i'll change.
> 
> > ffs_*
> > 	should just call ffs_* directly instead of UFS_*.
> > 
> > lfs_*
> > 	should just call lfs_* directly instead of UFS_*.
> 
> i think it's a matter of style.  but ok, i'll change them.

calling UFS_* is an indication that the vnode might be any of the
UFS types, but in this code we know which one it is, so I think
it's clearer to use the real function directly, so that someone
reading the code can be sure of the vnode's type as well.
but as you say, it's a matter of opinion.

-Chuck