mbox series

[0/4] remove (more) env callback code for SPL

Message ID 20200227135600.28853-1-rasmus.villemoes@prevas.dk
Headers show
Series remove (more) env callback code for SPL | expand

Message

Rasmus Villemoes Feb. 27, 2020, 1:56 p.m. UTC 
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(-)