Re: [PATCH 2/5] mm: remove unlikly NULL from kfree

From: Al Viro
Date: Thu Mar 26 2009 - 12:10:33 EST


On Wed, Mar 25, 2009 at 03:51:49PM +0200, Pekka Enberg wrote:
> OK, so according to Steven, audit_syscall_exit() is one such call-site
> that shows up in the traces. I don't really understand what's going on
> there but if it is sane, maybe that warrants the removal of unlikely()
> from kfree(). Hmm?

*Shrug*

We certainly can add explicit check, but that'll keep asking for
patches "removing useless check". The same applies to other places,
really.

And making kfree(NULL) break will keep reintroducing bugs, since people
will expect the behaviour of free(3)...

I don't have any serious objections to adding a check there; indeed,
the normal case there (
if (context->state != AUDIT_RECORD_CONTEXT) {
kfree(context->filterkey);
context->filterkey = NULL;
}
) is context->state != AUDIT_RECORD_CONTEXT, context->filterkey == NULL,
so it might be worth turning into
if (unlikely(context->filterkey)) {
if (context->state != AUDIT_RECORD_CONTEXT) {
kfree(context->filterkey);
context->filterkey = NULL;
}
}
anyway. Just dubious about the goal in general...
--
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/