Messages in this thread | | | Date | Tue, 27 Oct 2009 00:59:20 +0100 | From | Tilman Schmidt <> | Subject | Re: [PATCH 6/9] ser_gigaset: checkpatch cleanup |
| |
Am 26.10.2009 01:54 schrieb Joe Perches: > On Sun, 2009-10-25 at 20:30 +0100, Tilman Schmidt wrote: >> Duly uglified as demanded by checkpatch.pl. >> diff --git a/drivers/isdn/gigaset/ser-gigaset.c b/drivers/isdn/gigaset/ser-gigaset.c >> index 3071a52..ac3409e 100644 >> --- a/drivers/isdn/gigaset/ser-gigaset.c >> +++ b/drivers/isdn/gigaset/ser-gigaset.c >> @@ -164,9 +164,15 @@ static void gigaset_modem_fill(unsigned long data) >> { >> struct cardstate *cs = (struct cardstate *) data; >> struct bc_state *bcs; >> + struct sk_buff *nextskb; >> int sent = 0; >> >> - if (!cs || !(bcs = cs->bcs)) { >> + if (!cs) { >> + gig_dbg(DEBUG_OUTPUT, "%s: no cardstate", __func__); >> + return; >> + } >> + bcs = cs->bcs; >> + if (!bcs) { >> gig_dbg(DEBUG_OUTPUT, "%s: no cardstate", __func__); >> return; >> } > > perhaps: > if (!cs || !cs->bcs) { > gig_dbg(DEBUG_OUTPUT, "%s: no cardstate", __func__); > return; > } > bcs = cs->bcs;
That would evaluate cs->bcs twice, and is also, in my experience, significantly more prone to easily overlooked typos which result in checking a different pointer in the if statement than the one that's actually used in the subsequent assignment.
>> @@ -404,16 +412,20 @@ static void gigaset_device_release(struct device *dev) >> static int gigaset_initcshw(struct cardstate *cs) >> { >> int rc; >> + struct ser_cardstate *scs; >> >> - if (!(cs->hw.ser = kzalloc(sizeof(struct ser_cardstate), GFP_KERNEL))) { >> + scs = kzalloc(sizeof(struct ser_cardstate), GFP_KERNEL); >> + if (!scs) { >> pr_err("out of memory\n"); >> return 0; >> } >> + cs->hw.ser = scs; > > Why not no temporary and just: > > cs->hw.ser = kzalloc... > if (!cs->hw.ser)
For the same reasons as above.
Thanks, Tilman
-- Tilman Schmidt E-Mail: tilman@imap.cc Bonn, Germany Diese Nachricht besteht zu 100% aus wiederverwerteten Bits. Ungeöffnet mindestens haltbar bis: (siehe Rückseite)
[unhandled content-type:application/pgp-signature] | |