public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: "Bjørn Mork" <bjorn@mork.no>
To: Nicholas Mc Guire <der.herr@hofr.at>
Cc: Michal Marek <mmarek@suse.cz>,
	Julia Lawall <Julia.Lawall@lip6.fr>,
	Gilles Muller <Gilles.Muller@lip6.fr>,
	Nicolas Palix <nicolas.palix@imag.fr>,
	cocci@systeme.lip6.fr, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] scripts: coccinelle: kfree with TRACE_RET before ref is ok (resent)
Date: Tue, 22 Jul 2014 15:51:25 +0200	[thread overview]
Message-ID: <87ha298psy.fsf@nemi.mork.no> (raw)
In-Reply-To: <20140722101217.GA16183@opentech.at> (Nicholas Mc Guire's message of "Tue, 22 Jul 2014 12:12:17 +0200")

Nicholas Mc Guire <der.herr@hofr.at> writes:

> kfree.cocci currently triggers on constructs like
> (resending with proper CC list and subject line)
>
> drivers/staging/rts5208/spi.c
> 596    if (retval < 0) {
> 597            kfree(buf);
> 598            rtsx_clear_spi_error(chip);
> 599            spi_set_err_code(chip, SPI_HW_ERR);
> 600            TRACE_RET(chip, STATUS_FAIL);
> 601    }
> 602
> 603    rtsx_stor_access_xfer_buf(buf, pagelen, srb, &index, &offset,
>                             TO_XFER_BUF);
>
> with:
>
> ./drivers/staging/rts5208/spi.c:603:28-31: ERROR: reference preceded 
> by free on line 597
>
> but this should be fine - so TRACE_RET is added to the list of calls 
> "protecting" access to freed objects see drivers/staging/rts5208/trace.h

The TRACE_RET macro is a serious coding style violation. Fix that and
the error will go away.  Don't change scripts to hide issues with the
code. The code above won't just trigger an error in kfree.cocci but also
in the head of any sane person.

>From Documentation/CodingStyle:

<quote>
Things to avoid when using macros:

1) macros that affect control flow:

#define FOO(x)                                  \
        do {                                    \
                if (blah(x) < 0)                \
                        return -EBUGGERED;      \
        } while(0)

is a _very_ bad idea.  It looks like a function call but exits the "calling"
function; don't break the internal parsers of those who will read the code.
</quote>


Bjørn

      reply	other threads:[~2014-07-22 13:52 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-07-22 10:12 [PATCH] scripts: coccinelle: kfree with TRACE_RET before ref is ok (resent) Nicholas Mc Guire
2014-07-22 13:51 ` Bjørn Mork [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87ha298psy.fsf@nemi.mork.no \
    --to=bjorn@mork.no \
    --cc=Gilles.Muller@lip6.fr \
    --cc=Julia.Lawall@lip6.fr \
    --cc=cocci@systeme.lip6.fr \
    --cc=der.herr@hofr.at \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mmarek@suse.cz \
    --cc=nicolas.palix@imag.fr \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox