Re: clone() <-> getpid() bug in 2.6?

[Linus Torvalds]

On Sun, 6 Jun 2004, Denis Vlasenko wrote:
> >
> > 	if (now() - when < ((60 + (getpid() & 31)) << 6))
> 
> timeout randomization across several qmail-send processes, to prevent
> stampede effect?
> 
> > And don't you find
> > the above (totally uncommented) line just a thing of beauty and clarity?
> 
> Yes, a comment would be in place. :(

The problem is not so much what it does, but how it's coded.

For example, if it really wanted to do this, then how about having a nice 
variable that is initialized at run-time (preferably just once at 
startup), and has a nice name and a comment on what it's all about? Or 
even a macro, to make it more readable? Or even _just_ the comment?

You could write the above incomprehensible one-liner as

	/* Timeout in seconds: 64 - 97 minutes, depending on pid */
	#define ERROR_FAILURE_TIMEOUT ((60 + (getpid() & 31)) << 6)

	...

	/*
	 * If this IP address had a SMTP connection timeout last
	 * time, don't let it try to connect again immediately
	 */
	if (now() - when < ERROR_FAILURE_TIMEOUT)

and it would at least have made some sense.

As it is, you CANNOT read "tcpto.c" and make any sense of it. It's not 
sensible code. You have to know what the hell the whole thing is all 
about, and you have to actually try to figure out what the expression is 
to even understand the code.

Trust me. Try sometime. The code is horrible.

It wouldn't irritate me so much, but then the person who wrote that 
abomination has the galls to complain about sendmail, NFS, System V etc.

And it's not like qmail is so big and has such a long history that it 
would be a huge deal to comment it and write it more readably. But since 
the license doesn't allow for anything but patches, and patches would be 
totally unmaintainable if they tried to fix these kinds of things, it will 
never be done.

Which is a pity, since I actually _agree_ with you that qmail has some
good sides: it's small, it's efficient, and it's pretty modular. It has 
the _potential_ to be great code.

			Linus
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/