On Mon, Dec 15, 2014 at 08:29:24AM +0000, Stefan Schmidt wrote: > Hello. > > On 15/12/14 00:20, Alexander Aring wrote: > >This patch removes an unnecessary if branch inside the tx complete > >handler. > > > >Signed-off-by: Alexander Aring <alex.aring@xxxxxxxxx> > >--- > > drivers/net/ieee802154/at86rf230.c | 5 +---- > > 1 file changed, 1 insertion(+), 4 deletions(-) > > > >diff --git a/drivers/net/ieee802154/at86rf230.c > >b/drivers/net/ieee802154/at86rf230.c > >index 1c01356..4e983b3 100644 > >--- a/drivers/net/ieee802154/at86rf230.c > >+++ b/drivers/net/ieee802154/at86rf230.c > >@@ -715,10 +715,7 @@ at86rf230_tx_complete(void *context) > > > > enable_irq(lp->spi->irq); > > > >- if (lp->max_frame_retries <= 0) > >- ieee802154_xmit_complete(lp->hw, skb, true); > >- else > >- ieee802154_xmit_complete(lp->hw, skb, false); > >+ ieee802154_xmit_complete(lp->hw, skb, lp->max_frame_retries <= 0); > > } > > It surely saves us some lines but personally I find it harder to read this > way. Having the condition inside the function call breaks the reading flow > for me. Is this the preferred coding style in the kernel? > I agree this is more readable, maybe this is because I wrote this at first time in this way. But that reminds me to doing something like this: if (bool) return true; else return false; instead: return bool; which is usually some beginners failure. I don't know if the kernel conding style really discuss about that. The checkpatch script doesn't report anything about this. Maybe we can put something on the stack like: bool ifs_handling = lp->max_frame_retries <= 0; ... ieee802154_xmit_complete(lp->hw, skb, ifs_handling); is that more readable? - Alex -- To unsubscribe from this list: send the line "unsubscribe linux-wpan" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
