Re: [PATCH 9/8] lib/sort: turn off self-test

From: Paul Jackson (pj_at_sgi.com)
Date: 01/31/05

  • Next message: Hugh Dickins: "Re: Kernel bug: mm/rmap.c:483 and related {now 2.6.8}"
    Date:	Mon, 31 Jan 2005 12:49:57 -0800
    To: Matt Mackall <mpm@selenic.com>
    
    

    Matt wrote:
    > I actually think that more code
    > ought to have such tests, so long as they don't obscure the code in
    > question.

    If all the little unit tests that had ever been coded for kernel code
    were embedded in "#if 0 ... #endif" sections, it _would_ obscure the
    code.

    To the local maintainer of a file, it's not much of an issue, as they
    are actively familiar with what's there and look past the disabled test.
    But to the stranger passing through, it's just another three dozen lines
    of code that need to be waded through, or another false hit or two on
    some wide ranging grep that needs to be filtered out.

    Instead, I'd suggest a couple of comments:

     1) Describe this unit test in a comment, such as:

            /*
             * Nice unit test for this sort: try sorting the array
             * of 1000 integers constructed with the loop:
             * for (i = 0; i < 1000; i ++)
             * a[i] = r = (r * 725861) % 6599;
             * Sort then verify that for each i: a[i] <= a[i+1].
             */

        Five lines of comment are substantially less intrusive
        than three dozen lines of code. Both smaller, and even
        in the output of a grep, visually distinct.

     2) Describe whatever it was you stumbled on that's behind
        your comment:

            I ran into a strange regparm-related bug ...

    -- 
                      I won't rest till it's the best ...
                      Programmer, Linux Scalability
                      Paul Jackson <pj@sgi.com> 1.650.933.1373, 1.925.600.0401
    -
    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: Hugh Dickins: "Re: Kernel bug: mm/rmap.c:483 and related {now 2.6.8}"