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