On Saturday 19 August 2006 20:41, David Härdeman wrote: > I'm hoping to get some feedback on whether this is a good idea before I > commit anything. What is missing in the patch IMHO is a check if there is actually enough free memory to load the modules and run crypto before actually doing so. If you don't check, you still run the risk of crashing the installer. This could be a bit tricky as swap may be active at the time of the check and AFAIK it will be disabled during the actual operation. Not sure if that needs to be taken into account; a plain check for sufficient free memory would be a great start (maybe implemented as a function in di-utils). Also, is there a possibility that the function will be called again later? In that case it could be good to prevent running the memory check and loading the modules a second time. Finally, wasn't there some code that checked if everything was available before offering a method? Does that still work with this change? I like the fact that you only load the modules needed for the particular method selected. Nitpick: I feel that the name of the function "crypto_set_defaults" kind of fails to cover the udeb loading. Maybe crypto_medthod_prepare or something would be better. Cheers, FJP P.S. I'll look at the p-a-c code really soon now...
Attachment:
pgpbWVaOpWCV7.pgp
Description: PGP signature