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

From: Linus Torvalds (torvalds_at_osdl.org)
Date: 06/06/04

  • Next message: Randy.Dunlap: "Re: [PATCH] 2.6.6 memory allocation checks in SliceBlock()"
    Date:	Sun, 6 Jun 2004 10:55:08 -0700 (PDT)
    To: Denis Vlasenko <vda@port.imtp.ilyichevsk.odessa.ua>
    
    

    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@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/


  • Next message: Randy.Dunlap: "Re: [PATCH] 2.6.6 memory allocation checks in SliceBlock()"

    Relevant Pages

    • Re: Qmail
      ... I had a number of issues with Qmail. ... with all the patches proved to be too much work. ... > problem 'Disallowed breakage found in header name - potential virus' found ... > To unsubscribe, ...
      (freebsd-isp)
    • Re: Need to build a new mail server
      ... admins applying patches that they do not understand is a bad thing. ... That so much time and effort is spent telling everyone how bad qmail is ... experience running a commercial mail server. ... Most experienced sysadmins, once they know what they want, can apply those patches to qmail with ease and roll out additional Qmail installations with a single package. ...
      (freebsd-questions)
    • Re: [Lhms-devel] [PATCH 0/7] Fragmentation Avoidance V19
      ... > Sorry this wasn't meant to be a dig at your patches - I guess it turned ... where I went wrong because regression performance is wrong. ... it'll work just fine" fashion. ... send the line "unsubscribe linux-kernel" in ...
      (Linux-Kernel)
    • Re: kernel.bkbits.net off the air
      ... they could be used to recreate the whole repository in whatever format ... I guess it's just a matter of importing them like patches into arch. ... I did use the CVS gateway before to have a base tree to verify that the ... send the line "unsubscribe linux-kernel" in ...
      (Linux-Kernel)
    • Re: [PATCH] do_wait fix for 2.6.10-rc1
      ... almost certainly requires that a threaded app waits for its children in ... But maybe it's because I just missed some reason why this all is ok. ... three patches for the same piece of code withing minutes. ... send the line "unsubscribe linux-kernel" in ...
      (Linux-Kernel)