code cleanup
John Baldwin
jhb at FreeBSD.org
Thu Apr 29 11:11:36 PDT 2004
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 at FreeBSD.org> <>< http://www.FreeBSD.org/~jhb/
"Power Users Use the Power to Serve" = http://www.FreeBSD.org
More information about the freebsd-current
mailing list