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

On Wed, 2010-09-01 at 12:07 -0400, Miloslav Trmac wrote: 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