[PATCH] saa7146: fix sparse warnings - Kernel

This is a discussion on [PATCH] saa7146: fix sparse warnings - Kernel ; Went along with the existing file style. drivers/media/common/saa7146_core.c:309:6: warning: Using plain integer as NULL pointer drivers/media/common/saa7146_core.c:311:8: warning: Using plain integer as NULL pointer drivers/media/common/saa7146_core.c:319:7: warning: Using plain integer as NULL pointer drivers/media/common/saa7146_core.c:319:28: warning: Using plain integer as NULL pointer drivers/media/common/saa7146_core.c:325:7: ...

+ Reply to Thread
Results 1 to 7 of 7

Thread: [PATCH] saa7146: fix sparse warnings

  1. [PATCH] saa7146: fix sparse warnings

    Went along with the existing file style.
    drivers/media/common/saa7146_core.c:309:6: warning: Using plain integer as NULL pointer
    drivers/media/common/saa7146_core.c:311:8: warning: Using plain integer as NULL pointer
    drivers/media/common/saa7146_core.c:319:7: warning: Using plain integer as NULL pointer
    drivers/media/common/saa7146_core.c:319:28: warning: Using plain integer as NULL pointer
    drivers/media/common/saa7146_core.c:325:7: warning: Using plain integer as NULL pointer
    drivers/media/common/saa7146_core.c:325:28: warning: Using plain integer as NULL pointer
    drivers/media/common/saa7146_fops.c:275:12: warning: Using plain integer as NULL pointer

    Signed-off-by: Harvey Harrison
    ---
    drivers/media/common/saa7146_core.c | 8 ++++----
    drivers/media/common/saa7146_fops.c | 2 +-
    2 files changed, 5 insertions(+), 5 deletions(-)

    diff --git a/drivers/media/common/saa7146_core.c b/drivers/media/common/saa7146_core.c
    index 168a8d3..8c755a1 100644
    --- a/drivers/media/common/saa7146_core.c
    +++ b/drivers/media/common/saa7146_core.c
    @@ -306,9 +306,9 @@ static irqreturn_t interrupt_hw(int irq, void *dev_id)
    return IRQ_NONE;
    }

    - if( 0 != (dev->ext)) {
    + if( NULL != (dev->ext)) {
    if( 0 != (dev->ext->irq_mask & isr )) {
    - if( 0 != dev->ext->irq_func ) {
    + if( NULL != dev->ext->irq_func ) {
    dev->ext->irq_func(dev, &isr);
    }
    isr &= ~dev->ext->irq_mask;
    @@ -316,13 +316,13 @@ static irqreturn_t interrupt_hw(int irq, void *dev_id)
    }
    if (0 != (isr & (MASK_27))) {
    DEB_INT(("irq: RPS0 (0x%08x).\n",isr));
    - if( 0 != dev->vv_data && 0 != dev->vv_callback) {
    + if( NULL != dev->vv_data && NULL != dev->vv_callback) {
    dev->vv_callback(dev,isr);
    }
    isr &= ~MASK_27;
    }
    if (0 != (isr & (MASK_28))) {
    - if( 0 != dev->vv_data && 0 != dev->vv_callback) {
    + if( NULL != dev->vv_data && NULL != dev->vv_callback) {
    dev->vv_callback(dev,isr);
    }
    isr &= ~MASK_28;
    diff --git a/drivers/media/common/saa7146_fops.c b/drivers/media/common/saa7146_fops.c
    index f0703d8..a73db79 100644
    --- a/drivers/media/common/saa7146_fops.c
    +++ b/drivers/media/common/saa7146_fops.c
    @@ -272,7 +272,7 @@ static int fops_open(struct inode *inode, struct file *file)

    result = 0;
    out:
    - if( fh != 0 && result != 0 ) {
    + if( fh != NULL && result != 0 ) {
    kfree(fh);
    file->private_data = NULL;
    }
    --
    1.5.4.2.200.g99e75



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

  2. Re: [PATCH] saa7146: fix sparse warnings

    On Thu, 2008-02-21 at 21:19 +0000, Al Viro wrote:
    > On Thu, Feb 21, 2008 at 09:52:36AM -0800, Harvey Harrison wrote:
    >
    > Could you please use more descriptive names? NULL noise removal
    > is not the same as shadowing or endianness annotations or endianness
    > fixes or __user/__iomem annotations/fixes, etc.


    Point taken, will do so going forward.

    For now, I'm just doing the trivial ones to make the more interesting
    warnings stand out a little better.

    Harvey

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

  3. Re: [PATCH] saa7146: fix sparse warnings

    On Thu, Feb 21, 2008 at 09:52:36AM -0800, Harvey Harrison wrote:

    Could you please use more descriptive names? NULL noise removal
    is not the same as shadowing or endianness annotations or endianness
    fixes or __user/__iomem annotations/fixes, etc.
    --
    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/

  4. Re: [PATCH] saa7146: fix sparse warnings

    Harvey Harrison :
    > Went along with the existing file style.
    > drivers/media/common/saa7146_core.c:309:6: warning: Using plain integer as NULL pointer
    > drivers/media/common/saa7146_core.c:311:8: warning: Using plain integer as NULL pointer
    > drivers/media/common/saa7146_core.c:319:7: warning: Using plain integer as NULL pointer
    > drivers/media/common/saa7146_core.c:319:28: warning: Using plain integer as NULL pointer
    > drivers/media/common/saa7146_core.c:325:7: warning: Using plain integer as NULL pointer
    > drivers/media/common/saa7146_core.c:325:28: warning: Using plain integer as NULL pointer
    > drivers/media/common/saa7146_fops.c:275:12: warning: Using plain integer as NULL pointer
    >
    > Signed-off-by: Harvey Harrison
    > ---
    > drivers/media/common/saa7146_core.c | 8 ++++----
    > drivers/media/common/saa7146_fops.c | 2 +-
    > 2 files changed, 5 insertions(+), 5 deletions(-)
    >
    > diff --git a/drivers/media/common/saa7146_core.c b/drivers/media/common/saa7146_core.c
    > index 168a8d3..8c755a1 100644
    > --- a/drivers/media/common/saa7146_core.c
    > +++ b/drivers/media/common/saa7146_core.c
    > @@ -306,9 +306,9 @@ static irqreturn_t interrupt_hw(int irq, void *dev_id)
    > return IRQ_NONE;
    > }
    >
    > - if( 0 != (dev->ext)) {
    > + if( NULL != (dev->ext)) {


    At the risk of looking an idiot, I'm taking a liberty to ask what is
    the point in explicit comparison to zero in conditional operators? Is
    it not a fundamental C idiom to write

    if (a) {
    }

    instead of

    if (a != 0) {
    }

    and, similarly, to write

    if (!a) {
    }

    instead of

    if (a == 0) {
    }

    ?

    Thanks,

    Dmitri


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

  5. Re: [PATCH] saa7146: fix sparse warnings

    On Sun, 2008-03-02 at 20:34 +0300, Dmitri Vorobiev wrote:
    > Harvey Harrison пишет:
    > >
    > > - if( 0 != (dev->ext)) {
    > > + if( NULL != (dev->ext)) {

    >
    > At the risk of looking an idiot, I'm taking a liberty to ask what is
    > the point in explicit comparison to zero in conditional operators? Is
    > it not a fundamental C idiom to write




    Yes, that's how I would have written it, but I tried to keep with the
    prevailing style in that file. I suppose I could see an argument for
    consistency if you had a long series of if() statements to keep a
    similar style.

    if (foo == value1)

    if (bar == value2)

    if (baz == NULL)

    I'll leave the discussion of putting the constant first in the comparison
    for someone else to comment on.

    Harvey

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

  6. Re: [PATCH] saa7146: fix sparse warnings

    Harvey Harrison пишет:
    > On Sun, 2008-03-02 at 20:34 +0300, Dmitri Vorobiev wrote:
    >> Harvey Harrison пишет:
    >>>
    >>> - if( 0 != (dev->ext)) {
    >>> + if( NULL != (dev->ext)) {

    >> At the risk of looking an idiot, I'm taking a liberty to ask what is
    >> the point in explicit comparison to zero in conditional operators? Is
    >> it not a fundamental C idiom to write

    >
    >
    >
    > Yes, that's how I would have written it, but I tried to keep with the
    > prevailing style in that file. I suppose I could see an argument for
    > consistency if you had a long series of if() statements to keep a
    > similar style.


    To me it looks that the original style in this file can be sacrificed
    in favor of

    1) satisfying the coding style rules of the kernel;
    2) keeping with informal, but commonly known idioms of C language.

    So I thought that while you're at it, you could also comb through
    this file and make the checkpatch.pl script happy with it.

    But this is just my very personal opinion

    Dmitri

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

  7. Re: [PATCH] saa7146: fix sparse warnings

    On Sun, 02 Mar 2008 10:06:44 -0800 Harvey Harrison wrote:

    > On Sun, 2008-03-02 at 20:34 +0300, Dmitri Vorobiev wrote:
    > > Harvey Harrison пишет:
    > > >
    > > > - if( 0 != (dev->ext)) {
    > > > + if( NULL != (dev->ext)) {

    > >
    > > At the risk of looking an idiot, I'm taking a liberty to ask what is
    > > the point in explicit comparison to zero in conditional operators? Is
    > > it not a fundamental C idiom to write

    >
    >
    >
    > Yes, that's how I would have written it, but I tried to keep with the
    > prevailing style in that file.


    I'm not a bit fan of the match-the-existing-style approach.

    You'll find that files which started out with a non-standard style already
    contain a mixup of styles, because later changes were often made in
    standard-style.

    And given that we're churning the code anyway, we might as well fix it up now.
    Yes, that'll make the code look partially-weird rather than wholly-weird,
    but I doubt if that will harm anyone much.

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

+ Reply to Thread