Date: Thu, 29 Apr 2004 07:06:43 +0300 From: Alex Lyashkov <shadow@psoft.net> To: John Baldwin <jhb@FreeBSD.org> Cc: Julian Elischer <julian@elischer.org> Subject: Re: code cleanup Message-ID: <1083211603.8246.40.camel@berloga.shadowland> In-Reply-To: <200404281541.08851.jhb@FreeBSD.org> References: <Pine.BSF.4.21.0404281120480.73191-100000@InterJet.elischer.org> <200404281541.08851.jhb@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
=F7 =F3=D2=C4, 28.04.2004, =D7 22:41, John Baldwin =D0=C9=DB=C5=D4:
> On Wednesday 28 April 2004 02:26 pm, Julian Elischer wrote:
> > On Wed, 28 Apr 2004, John Baldwin wrote:
> > > On Wednesday 28 April 2004 02:26 am, Alex Lyashkov wrote:
> > > > Hi All
> > > >
> > > > how i see many points at kernel work with allproc list direct, but
> > > > proc.h introduce macros FOREACH_PROC_IN_SYSTEM.
> > > > This patch clean this places.
> > >
> > > I'd actually rather see the FOREACH_PROC macro removed, I don't think
> > > hiding the fact that it's a TAILQ is all that useful.
> >
> > it makes it possible (well, easier) to do:
> >
> > FOREACH_PROC_IN_SYSTEM(p) {
> > FOREACH_KSEGROUP_IN_PROC(p, kg) {
> > FOREACH_THREAD_IN_GROUP(kg.td) {
> > something(td, kg);
> > }
> > }
> > }
> >
> > Which is a lot easier to read and understand
> > than the expanded version. You don't kave to remember the linkage
> > pointer's names and you can add debugging to it
> > and check that the correct loks are held etc.
> > (the latter being a major reason I did it).
>=20
> Note that the allproc_lock protects the allproc list. W/o the FOREACH_PR=
OC=20
> macro, I can grep for 'allproc' in the source tree to find all users to=20
> verify locking, etc. With the extra macro, I now have to do multiple gre=
ps.=20
two greps is multiple ? first of FOREACH_PROC, second allproc or combine
at one grep with two -e parameters.
> When you multiple the effect with several wrapper macros, it now becomes =
much=20
> more work to work on locking the lists of structures since you have to do=
=20
> multiple greps to find the places to look at. I think remembering the=20
> linkages for lists is actually quite important to avoid using the same=20
> 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.
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D
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);
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D
where difficult for find locking problem? but using macro allow write
more readable code.
Other argument for this patch - FreeBSD already have this macro and src
need change to even code.
--=20
Alex Lyashkov <shadow@psoft.net>
PSoft
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?1083211603.8246.40.camel>