cvs commit: src/sys/net radix.c
max at love2party.net
Wed Apr 21 17:58:40 PDT 2004
On Thursday 22 April 2004 00:21, Darren Reed wrote:
> On Thu, Apr 22, 2004 at 01:03:42AM +0100, Colin Percival wrote:
> > At 00:24 22/04/2004, Darren Reed wrote:
> > >Some of these changes, for the sake of change, can be annoying...
> > As Luigi mentioned when he started this, these aren't changes "for the
> > sake of change"; rather, these are changes to clean things up so that it
> > is easier to add all the necessary locking.
> Uh, really?
> Changing variable names is required to make locking easier?
> Changing "x = *(u_char *)p" into "x = LEN(p)" makes locking easier?
In the end: Yes it does! In this particular example I can easily tell what the
new version does, while I wouldn't be able to tell it from the first (w/o
context). It just way easier to read, keeping it obscure as all the other
BSDs have it this way and it was this way for years is just baloney.
To put some emphasis on why this will help locking, we don't have many brave
who happen to know <your_network_subsystem_here> from top to bottom and can
easily tell what "x = *(u_char *)p" is supposed to mean, but we have some who
have a good idea how to do locking. The changes Luigi is trying to do, should
enable better readability of the network code to make it possible for others
to lock things down.
I find readable code a good thing (even if it does not help locking).
> In the end, if someone decides that the code needs to be restructured
> for better performance in SMP hardware, then that's what'll happen and
> changes like these will have little significance at that point in time.
I don't think so. It's much easier to restructure code you can actually read
and understand. And honest, when one sees the current radix code (etc.), the
first thing that comes to mind is "run!". Same applies to IPv6/KAME-code, btw
- and I didn't see anybody to work on the locking there and I know a reason
Best regards, | mlaier at freebsd.org
Max Laier | ICQ #67774661
http://pf4freebsd.love2party.net/ | mlaier at EFnet
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Size: 187 bytes
Url : http://lists.freebsd.org/pipermail/cvs-all/attachments/20040422/9a048300/attachment.bin
More information about the cvs-all