From owner-freebsd-current@FreeBSD.ORG Thu Apr 29 11:11:36 2004 Return-Path: Delivered-To: freebsd-current@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 8C77816A4D0 for ; Thu, 29 Apr 2004 11:11:36 -0700 (PDT) Received: from mail3.speakeasy.net (mail3.speakeasy.net [216.254.0.203]) by mx1.FreeBSD.org (Postfix) with ESMTP id 6823343D5E for ; Thu, 29 Apr 2004 11:11:36 -0700 (PDT) (envelope-from jhb@FreeBSD.org) Received: (qmail 12794 invoked from network); 29 Apr 2004 18:11:35 -0000 Received: from dsl027-160-063.atl1.dsl.speakeasy.net (HELO server.baldwin.cx) ([216.27.160.63]) (envelope-sender ) encrypted SMTP for ; 29 Apr 2004 18:11:35 -0000 Received: from 10.50.40.205 (gw1.twc.weather.com [216.133.140.1]) by server.baldwin.cx (8.12.11/8.12.11) with ESMTP id i3TIBVbM023218; Thu, 29 Apr 2004 14:11:33 -0400 (EDT) (envelope-from jhb@FreeBSD.org) From: John Baldwin To: freebsd-current@FreeBSD.org Date: Thu, 29 Apr 2004 11:24:30 -0400 User-Agent: KMail/1.6 References: <200404281541.08851.jhb@FreeBSD.org> <1083211603.8246.40.camel@berloga.shadowland> In-Reply-To: <1083211603.8246.40.camel@berloga.shadowland> MIME-Version: 1.0 Content-Disposition: inline Message-Id: <200404291120.39967.jhb@FreeBSD.org> Content-Type: text/plain; charset="koi8-r" Content-Transfer-Encoding: 7bit X-Spam-Checker-Version: SpamAssassin 2.63 (2004-01-11) on server.baldwin.cx cc: Alex Lyashkov cc: Julian Elischer Subject: Re: code cleanup X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 29 Apr 2004 18:11:36 -0000 On Thursday 29 April 2004 12:06 am, Alex Lyashkov wrote: > > Note that the allproc_lock protects the allproc list. W/o the > > FOREACH_PROC macro, I can grep for 'allproc' in the source tree to find > > all users to verify locking, etc. With the extra macro, I now have to do > > multiple greps. > > two greps is multiple ? first of FOREACH_PROC, second allproc or combine > at one grep with two -e parameters. Multiple means more than one, yes. When I'm searching the tree when locking a structure or fields of a structure I don't usually come up with complex grep statements, and actually, I wouldn't find the FOREACH_FOO macro until I did the first grep anyway. When you add lots of macros that do this you get a compounding problem. > > When you multiple the effect with several wrapper macros, it now becomes > > much more work to work on locking the lists of structures since you have > > to do multiple greps to find the places to look at. I think remembering > > the linkages for lists is actually quite important to avoid using the > > same linkage for multiple lists incorrectly. > > you wrong. > it`s code from linux kernel, but illustrate who create more readable > code with macros same as FORAEACH_PROC_IN_SYSTEM. > ========================== > read_lock(&in_dev->lock); > for_primary_ifa(in_dev) { > if (inet_ifa_match(a, ifa)) { > if (!b || inet_ifa_match(b, ifa)) { > read_unlock(&in_dev->lock); > return 1; > } > } > } endfor_ifa(in_dev); > read_unlock(&in_dev->lock); > ========================== > where difficult for find locking problem? but using macro allow write > more readable code. This is not the same type of deal. In this case, the object being locked 'in_dev' is passed as an argument to the macro, so a grep for 'in_dev' still turns up this line. A comparable change would be to have FOREACH_PROC_IN_SYSTEM() be 'FOREACH_PROC(p, &allprocs)' in which case grep for allprocs finds the line. That actually matches the style of the other macros that all take two arguments: an iterator and a source of the list. Note that such a change would also allow the macro to be used with the zombprocs list as well. I'm not convinced that FOREACH_PROC(p, &allprocs) is all that different from: TAILQ_FOREACH(p, &allprocs, p_list) though. -- John Baldwin <>< http://www.FreeBSD.org/~jhb/ "Power Users Use the Power to Serve" = http://www.FreeBSD.org