Re: [PATCH] don't do pointless NULL checks and casts before kfree() in security/
From: Ralph Corderoy
Date: Mon Mar 21 2005 - 05:18:42 EST
Hi Jesper,
> > > the short version also have the real bennefits of generating
> > > shorter and faster code as well as being shorter "on-screen".
> >
> > Faster code? I'd have thought avoiding the function call outweighed
> > the overhead of checking before calling.
>
> I haven't actually measured it, but that would be my guess from
> looking at the actual code gcc generates.
> ...
> void cond_policydb_destroy(struct policydb *p)
> {
> 220: 55 push %ebp
> 221: 89 e5 mov %esp,%ebp
> 223: 53 push %ebx
> 224: 89 c3 mov %eax,%ebx
> if (p->bool_val_to_struct)
> 226: 8b 40 78 mov 0x78(%eax),%eax
> 229: 85 c0 test %eax,%eax
> 22b: 75 13 jne 240 <cond_policydb_destroy+0x20>
> kfree(p->bool_val_to_struct);
> avtab_destroy(&p->te_cond_avtab);
> 22d: 8d 43 7c lea 0x7c(%ebx),%eax
> 230: e8 fc ff ff ff call 231 <cond_policydb_destroy+0x11>
> cond_list_destroy(p->cond_list);
> 235: 8b 83 84 00 00 00 mov 0x84(%ebx),%eax
> 23b: 5b pop %ebx
> 23c: c9 leave
> 23d: eb c1 jmp 200 <cond_list_destroy>
> 23f: 90 nop
> 240: e8 fc ff ff ff call 241 <cond_policydb_destroy+0x21>
> 245: 8d 43 7c lea 0x7c(%ebx),%eax
> 248: e8 fc ff ff ff call 249 <cond_policydb_destroy+0x29>
> 24d: 8b 83 84 00 00 00 mov 0x84(%ebx),%eax
> 253: 5b pop %ebx
> 254: c9 leave
> 255: eb a9 jmp 200 <cond_list_destroy>
> 257: 89 f6 mov %esi,%esi
> 259: 8d bc 27 00 00 00 00 lea 0x0(%edi),%edi
>
> [...]
> ...
> First of all that's significantly shorter, so we'll gain a bit of
> memory and I'd guess it would improve cache behaviour as well (but I
> don't know enough to say for sure).
Yes, the original's awful isn't it. I'm used to ARM rather than x86 and
so didn't expect such bloat by having the condition.
> I'm also assuming that in the vast majority of cases (not just here,
> but all over the kernel) the pointer being tested will end up being
> !=NULL so we'll end up doing the function call in any case, so saving
> the conditional should be an overall win.
Fair enough, you've persuaded me. Thanks for taking the time.
Cheers,
Ralph.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/