lkml.org 
[lkml]   [2007]   [Sep]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 1/3] CodingStyle updates
    A couple of comments interspersed...

    On 9/28/07, Erez Zadok <ezk@cs.sunysb.edu> wrote:
    ...
    > There are a number of driver model diagnostic macros in <linux/device.h>
    > which you should use to make sure messages are matched to the right device
    ---

    I think this "which" is non-restrictive, so it should have a comma
    after it (I realize that's not part of your patch). It's also possible
    to read it as restrictive, in which case "that" would be preferable.

    ---
    > and driver, and are tagged with the right level: dev_err(), dev_warn(),
    > -dev_info(), and so forth. For messages that aren't associated with a
    > -particular device, <linux/kernel.h> defines pr_debug() and pr_info().
    > +dev_info(), and so forth.
    > +
    > +A number of people often like to define their own debugging printf's,
    ---

    "A number of people often like to..." is awkward. How about
    "Developers sometimes..." or "Too many people..."

    ---
    > +wrapping printk's in #ifdef's that get turned on only when subsystem

    > + Chapter 19: branch prediction optimizations
    > +
    > +The kernel includes macros called likely() and unlikely(), which can be used
    ---

    You might say "The kernel provides the macros likely()..."

    ---
    > +as hints to the compiler to optimize branch prediction. They operate by
    ...
    > +A good example of this is the above kmalloc(GFP_KERNEL) call. The chances
    > +of kmalloc() returning NULL are rather small, because even if it doesn't
    > +have memory to return to you at the moment, with GFP_KERNEL/__GFP_WAIT
    > +passed, kmalloc() will wait and suspend your thread, while it goes off to
    ---

    The commas after "passed" and "thread" is unnecessary.

    ---
    > +find some free memory (purging caches, flushing buffers, etc.). In other
    > +words, kmalloc() tries very hard to give you the memory you asked for by the
    > +time it return.
    ---

    "...by the time it return." should be "...by the time it returns."

    ---
    > +
    > +Consider the next, bad example. Suppose you're developing a file system
    > +which performs logically different actions on different types of entities:
    ---

    This "which" is restrictive; it would be preferable to use "that" instead.

    ---
    > +files, directories, symlinks, devices, etc. and you use this code:
    > +
    ...
    > +be penalized heavily for going [sic] down the wrong path... Therefore, you
    > +should consider also whether a seemingly-rare condition is indeed rare ALL
    ---

    The hyphen isn't necessary when the first word of the compount
    adjective is an adverb ending in "-ly", so, just "seemingly rare"; or
    switch to "apparently rare".

    ---
    > +the time.
    ...


    --
    scott preece
    -
    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/

    \
     
     \ /
      Last update: 2007-09-29 18:03    [W:0.022 / U:64.664 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site