Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 27 Aug 1999 16:18:45 -0400 (EDT)
From:      Erez Zadok <ezk@cs.columbia.edu>
To:        Matthew Dillon <dillon@apollo.backplane.com>
Cc:        Alfred Perlstein <bright@wintelcom.net>, hackers@FreeBSD.ORG, fs@FreeBSD.ORG, Michael Hancock <michaelh@cet.co.jp>, David Greenman <dg@root.com>
Subject:   Re: HEADS UP Reviewers. VFS changes to be committed. 
Message-ID:  <199908272018.QAA22373@shekel.mcl.cs.columbia.edu>
In-Reply-To: Your message of "Thu, 26 Aug 1999 10:27:47 PDT." <199908261727.KAA23308@apollo.backplane.com> 

next in thread | previous in thread | raw e-mail | index | archive | help
In message <199908261727.KAA23308@apollo.backplane.com>, Matthew Dillon writes:
[...]
>     I would ask two things though:
> 
> 	* First, please add comprehensive /* */ comments in front of each 
> 	  vfsnop_*() procedure explaining what it does, why it returns what
> 	  it returns, locking requirements (if any) on entry, and side effects
> 	  on return.  This is just for readability.
> 
> 	  Do the same for all the procedures you are adding, in fact.

Moreover, I would strongly recommend xplicitly documenting the following:

- which function args are in-args and which are out-args?

- does the function takes any allocated memory that it is expected to free?

- is the function expected to allocate any memory objects that have to be
  freed elsewhere?

- does the function increase or decrease any reference counts of any objects?
  Is it expected to?

These and other requirements are essentially the "interface" between the VFS
and lower-level file systems.  Figuring out this stuff on every OS and OS
revision (esp. when the VFS changes so often---linux) was the longest most
frustrating task I faced when writing my Wrapfs stackable f/s module for
solaris, freebsd, and linux.  I wish documentation had been in place.

> 	* I think you can safely commit any elements that are not used by
> 	  existing builds since they are not likely to impact existing
> 	  builds operationally.
> 
> 	  Then see what you have left over.  If it is not significant, commit
> 	  that to.  If it is significant, do more comprehensive testing on
> 	  what you have left over (i.e. that impacts existing builds) and
> 	  ask for another review after testing, before committing it.

Erez.


To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-hackers" in the body of the message




Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?199908272018.QAA22373>