Re: [patch RFC]: userspace crypto auditing, v2

[Eric Paris <eparis@redhat.com>]

On Wed, 2010-09-01 at 12:07 -0400, Miloslav Trmac wrote:
> Hello,
> I'm posting these patches for early review again; users of the code are not in the kernel yet.

I didn't really review them but I have just 2 style comments and one
thing you need to make sure of.  There are a couple of places the
indention starts to get way to the right.  I know there are places you
have things like

if (blah) {
	do blah
}
break;

and I'd typically rather see:

if (!blah)
	break;
do blah;
break;

A couple functions I think you can safely drop a level of indentation
include audit_log_crypto_op(), audit_filter_rules(), and maybe
log_crypto_op() needs a helper function to cut down the indentation?
Maybe not.

The other comment is that I disagree with the 80 column rule if and ONLY
if your code does not have too many levels of indention.  If you are
going to run just past 80 and only 2 or 3 levels of indentation go
ahead.  Do what looks best for the code.  I know Al likes 80 more than I
do, but doing stupid things just to meet 80 columns is foolish.

(This actually might be a topic at the upcoming kernel summit, so we
might have more hard and fast rules in the future, but for now, I
suggest trying to stick to 80, but if the code looks a lot better going
to 85 or maybe 90 or possibly even more, I say do it.

I really don't like %s in audit_log_format().  So unless its easy to
prove that the string meets all the rules and always will meet the
rules, please use audit_log_string()  (and in this code I noticed that I
could not verify 'operation' in this patch, which makes me very nervous.

I'll try to get some time to actually review the patch

-Eric