[lkml]   [2005]   [Mar]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
    SubjectRe: Do not misuse Coverity please (Was: sound/oss/cs46xx.c: fix a check after use)
    Jean Delvare <> wrote:
    > > This patch fixes a check after use found by the Coverity checker.
    > > (...)
    > > static void amp_hercules(struct cs_card *card, int change)
    > > {
    > > - int old=card->amplifier;
    > > + int old;
    > > if(!card)
    > > {
    > > CS_DBGOUT(CS_ERROR, 2, printk(KERN_INFO
    > > "cs46xx: amp_hercules() called before initialized.\n"));
    > > return;
    > > }
    > > + old = card->amplifier;
    > I see that you are fixing many bugs like this one today, all reported by
    > Coverity. In all cases (as far as I could see at least) you are moving
    > the dereference after the check. Of course it prevents any NULL pointer
    > dereference, and will make Coverity happy. However, I doubt that this is
    > always the correct solution.
    > Think about it. If the pointer could be NULL, then it's unlikely that
    > the bug would have gone unnoticed so far (unless the code is very
    > recent). Coverity found 3 such bugs in one i2c driver [1], and the
    > correct solution was to NOT check for NULL because it just couldn't
    > happen.

    No, there is a third case: the pointer can be NULL, but the compiler
    happened to move the dereference down to after the check.

    If the optimiser is later changed, or if someone tries to compile the code
    with -O0, it will oops.

    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to
    More majordomo info at
    Please read the FAQ at

     \ /
      Last update: 2005-04-06 13:31    [W:0.041 / U:30.108 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site