From owner-freebsd-fs Fri Aug 27 13:21:57 1999 Delivered-To: freebsd-fs@freebsd.org Received: from cs.columbia.edu (cs.columbia.edu [128.59.16.20]) by hub.freebsd.org (Postfix) with ESMTP id 836D81557C; Fri, 27 Aug 1999 13:21:35 -0700 (PDT) (envelope-from ezk@shekel.mcl.cs.columbia.edu) Received: from shekel.mcl.cs.columbia.edu (shekel.mcl.cs.columbia.edu [128.59.18.15]) by cs.columbia.edu (8.9.1/8.9.1) with ESMTP id QAA02879; Fri, 27 Aug 1999 16:18:47 -0400 (EDT) Received: (from ezk@localhost) by shekel.mcl.cs.columbia.edu (8.9.1/8.9.1) id QAA22373; Fri, 27 Aug 1999 16:18:45 -0400 (EDT) Date: Fri, 27 Aug 1999 16:18:45 -0400 (EDT) Message-Id: <199908272018.QAA22373@shekel.mcl.cs.columbia.edu> X-Authentication-Warning: shekel.mcl.cs.columbia.edu: ezk set sender to ezk@shekel.mcl.cs.columbia.edu using -f From: Erez Zadok To: Matthew Dillon Cc: Alfred Perlstein , hackers@FreeBSD.ORG, fs@FreeBSD.ORG, Michael Hancock , David Greenman Subject: Re: HEADS UP Reviewers. VFS changes to be committed. In-reply-to: Your message of "Thu, 26 Aug 1999 10:27:47 PDT." <199908261727.KAA23308@apollo.backplane.com> Sender: owner-freebsd-fs@FreeBSD.ORG Precedence: bulk X-Loop: FreeBSD.org 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-fs" in the body of the message