cvs commit: src/sys/net radix.c

[Max Laier]

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 
why.

-- 
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
Type: application/pgp-signature
Size: 187 bytes
Desc: signature
Url : http://lists.freebsd.org/pipermail/cvs-src/attachments/20040422/9a048300/attachment.bin