Re: [PATCH 9/10] drivers/mmc: Move a dereference below a NULL test

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



arun c wrote:
On Mon, Jul 20, 2009 at 1:31 PM, Adrian Hunter<adrian.hunter@xxxxxxxxx> wrote:
Julia Lawall wrote:
From: Julia Lawall <julia@xxxxxxx>

If the NULL test is necessary, then the dereference should be moved below
the NULL test.

The semantic patch that makes this change is as follows:
(http://www.emn.fr/x-info/coccinelle/)

// <smpl>
@@
type T;
expression E,E1;
identifier i,fld;
statement S;
@@

- T i = E->fld;
+ T i;
 ... when != E=E1
     when != i
 BUG_ON (E == NULL||
-     i
+     E->fld
      == NULL || ...);
+ i = E->fld;
// </smpl>

Signed-off-by: Julia Lawall <julia@xxxxxxx>

---
 drivers/mmc/host/omap.c             |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/host/omap.c b/drivers/mmc/host/omap.c
index e7a331d..89281ab 100644
--- a/drivers/mmc/host/omap.c
+++ b/drivers/mmc/host/omap.c
@@ -255,11 +255,12 @@ static void mmc_omap_slot_release_work(struct
work_struct *work)
  static void mmc_omap_release_slot(struct mmc_omap_slot *slot, int
clk_enabled)
 {
-       struct mmc_omap_host *host = slot->host;
+       struct mmc_omap_host *host;
       unsigned long flags;
       int i;
 -       BUG_ON(slot == NULL || host->mmc == NULL);
+       BUG_ON(slot == NULL || slot->host->mmc == NULL);
+       host = slot->host;
         if (clk_enabled)
               /* Keeps clock running for at least 8 cycles on valid freq
*/
--
If slot is NULL it will oops anyway, so the following is better IMHO:

static void mmc_omap_release_slot(struct mmc_omap_slot *slot, int
clk_enabled)
{
       struct mmc_omap_host *host = slot->host;
       unsigned long flags;
       int i;

-       BUG_ON(slot == NULL || host->mmc == NULL);
+       BUG_ON(host->mmc == NULL);
       if (clk_enabled)
               /* Keeps clock running for at least 8 cycles on valid freq */
--

According to http://isc.sans.org/diary.html?storyid=6820&rss "NULL check"
must be done before assigning the values. Does Julia Lawall's version
is the more correct one??

Arun

It is not a "NULL check" it is an assertion, which can be compiled out.
On some platforms BUG is implemented as a null dereference anyway
e.g. on ARM  it is *(int *)0 = 0;

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Arm (vger)]     [ARM Kernel]     [ARM MSM]     [Linux Tegra]     [Linux WPAN Networking]     [Linux Wireless Networking]     [Maemo Users]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux