Date: Thu, 29 Apr 2004 11:24:30 -0400 From: John Baldwin <jhb@FreeBSD.org> To: freebsd-current@FreeBSD.org Cc: Julian Elischer <julian@elischer.org> Subject: Re: code cleanup Message-ID: <200404291120.39967.jhb@FreeBSD.org> In-Reply-To: <1083211603.8246.40.camel@berloga.shadowland> References: <Pine.BSF.4.21.0404281120480.73191-100000@InterJet.elischer.org> <200404281541.08851.jhb@FreeBSD.org> <1083211603.8246.40.camel@berloga.shadowland>
next in thread | previous in thread | raw e-mail | index | archive | help
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 <jhb@FreeBSD.org> <>< http://www.FreeBSD.org/~jhb/
"Power Users Use the Power to Serve" = http://www.FreeBSD.org
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200404291120.39967.jhb>