From patchwork Thu Feb 27 13:56:09 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Rasmus Villemoes X-Patchwork-Id: 236948 List-Id: U-Boot discussion From: rasmus.villemoes at prevas.dk (Rasmus Villemoes) Date: Thu, 27 Feb 2020 13:56:09 +0000 Subject: [PATCH 0/4] remove (more) env callback code for SPL Message-ID: <20200227135600.28853-1-rasmus.villemoes@prevas.dk> The actual environment callbacks are automatically dropped from an SPL build, but the support code (env/callback.o) for associating a callback to an environment variable is still there. These patches reduce a CONFIG_SPL_ENV_SUPPORT=y SPL image by about 1K (at least for my ppc build), and another ~2K of run-time memory. === Aside: While poking around in this code, I noticed a few somewhat related bugs in callback.o: (1) The caching of env_get(".callbacks") in env_callback_init() is dubious at best: It gets called during the initial import, so env_get ends up calling env_get_f. So either we cache a value of NULL (no .callbacks variable in initial environment), or we cache a value of gd->env_buf. Now, of course env_get_f will soon no longer be used, so there's not a big risk that gd->env_buf will be overwritten, but it's rather fragile. In either case (2), the cache is not invalidated or updated by the on_callbacks() callback associated to .callbacks itself. Finally, (3) with CONFIG_REGEX=y, that on_callbacks() function has the unfortunate effect of removing itself as the callback associated to .callbacks: When .callbacks is initially added to the environment, env_callback_init() correctly associates on_callbacks, because it uses env_attr_lookup() (which is regex-aware) on the ENV_CALLBACK_STATIC_LIST. Now, when .callbacks is set the next time, on_callbacks() is called, starts by clearing all callbacks, including the one for .callbacks. It then tries to initialize them again, but it uses env_attr_walk() (which roughly speaking just splits the given string on , and : and calls back to the user's handler) on ENV_CALLBACK_STATIC_LIST, which means that set_callback() gets called with the string "\.callbacks" - and such a variable does not exist. set_callback() is never called with ".callbacks" as name, so .callbacks (and e.g. eth5addr - anything else which is supposed to be matched by a regex) now no longer has a callback. Perhaps this can all be fixed by just having on_callbacks update the cached static callback_list with value, then do a hwalk_r(&env_htab, env_callback_init) - no need for either set_callback or clear_callback. Rasmus Villemoes (4): env: remove callback.o for an SPL build lib/hashtable.c: create helper for calling env_entry::callback lib/hashtable.c: don't test ->callback in SPL make env_entry::callback conditional on !CONFIG_SPL_BUILD env/Makefile | 2 +- env/callback.c | 2 ++ env/flags.c | 1 - include/env_callback.h | 6 ++++++ include/search.h | 2 ++ lib/hashtable.c | 26 +++++++++++++++++--------- 6 files changed, 28 insertions(+), 11 deletions(-)