Hi Simon,
On 13.03.2015 03:48, Simon Glass wrote:
This patch adds the feature to only stop the autobooting, and therefor boot into the U-Boot prompt, when the input string / password matches a values that is encypted via a SHA256 hash and saved in the environment.
This feature is enabled by defined these config options: CONFIG_AUTOBOOT_KEYED CONFIG_AUTOBOOT_STOP_STR_SHA256
Signed-off-by: Stefan Roese sr@denx.de
This is certainly interesting but I think brings us back to a point Simon made a long while back about needing to factor out this code better. Especially since this adds big long #if-#else-#endif blocks. Can we re-do this so at least have some functions be called out instead? Thanks!
Also if these CONFIG options are in Kconfig (as they should be) then we can use
if (IS_ENABLED(CONFIG_AUTOBOOT_STOP_STR_SHA256))
instead of #ifdef which may improve the code.
Right. I also thought about this. But the resulting code has all the functionality extracted into 2 functions:
#if defined(CONFIG_AUTOBOOT_STOP_STR_SHA256) static int passwd_abort(uint64_t etime) { const char *sha_env_str = getenv("bootstopkeysha256"); ... } #else static int passwd_abort(uint64_t etime) { int abort = 0; ... } #endif
And this function is now called unconditionally:
... abort = passwd_abort(etime);
So there is nothing here that could be simplified by using IS_ENABLED().
I could of course just add this new config option to Kconfig. But with all the other related options not in Kconfig (CONFIG_AUTOBOOT_KEYED, CONFIG_AUTOBOOT_DELAY_STR...), this doesn't make much sense. So at some point all those config options should be moved to Kconfig. Unfortunately I don't have the time for this right now. But I'll add it to my list to do this at a later time.
Thanks, Stefan