Paul Jackson wrote:
nits:
1) I'm not sure the "no-op if CONFIG_CSA not set" comments
are worthwhile - it does not seem to be a common practice
to mark macros that collapse under certain CONFIG's with
such comments, and some code, such as in fork.c, would
become quite a bit less readable if such comments were
widely used.
Yeah, that makes sense. Will be fixed in next posting.
2) Three of the added csa_update_integrals() lines have
leading spaces, instead of a tab char, such as in:
===================================================================
--- linux.orig/fs/exec.c 2004-09-27 11:57:40.201435722 -0700
+++ linux/fs/exec.c 2004-09-27 14:05:41.266160725 -0700
@@ -1163,6 +1164,9 @@
/* execve success */
security_bprm_free(&bprm);
+ /* no-op if CONFIG_CSA not set */
+ csa_update_integrals(); <=========
+ update_mem_hiwater(); <=========
return retval;
}
Caused by 'cut-n-paste'. Will be fixed.
3) Is it always the case that csa_update_integrals() and
update_mem_hiwater() are used together? If so, perhaps
they could be collapsed into one? Even the current->mm
test inside them could be made one test, perhaps?
As Robin pointed out, there are a couple of instances that are
not the case. Actually there are three.
Thanks for your feedback, Paul!
- jay
|