scripts: coccinelle: kfree with TRACE_RET before ref is ok (resent)
PATCH 1/1 scripts: coccinelle: kfree with TRACE_RET before ref is ok (resent)
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
--- scripts/coccinelle/free/kfree.cocci | 2 ++ 1 file changed, 2 insertions(+)
scripts/coccinelle/free/kfree.cocci +2 -0
@@ -101,6 +101,8 @@ kfree at p1(E,...) | return_ACPI_STATUS(...) | + TRACE_RET(...) +| E at p2 // bad use )
Signature · 1 line
1.7.10.4
Quoted 14 lines · Nicholas Mc Guire
kfree.coccicurrently 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);
Quoted 7 lines
with:
./drivers/staging/rts5208/spi.c:603:28-31: ERROR: reference preceded by free on line 597
but this should be fine - so
TRACE_RETis added to the list of calls "protecting" access to freed objects seedrivers/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.
Quoted 1 line
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