Hi Serhei, On Sun, Mar 16, 2025 at 07:13:00PM -0400, Serhei Makarov wrote: > Renaming to dwfl_set_initial_registers_thread. > > This callback was private to one file, but now that other > tools (eu-stacktrace, sysprof via the dwfl_perf_sample_getframes) are > invoking the ebl set_initial_registers_sample api, we need to expose > it. Otherwise, clients would need to reimplement this code > identically, including the undocumented/unexplained use of -2 for > aarch64 insn_mask. > > TODO(REVIEW): Should this be in libdwflP.h since libebl is private? > On the other hand, dwfl_thread_state_register_pc et al. are also > public in spite of being used primarily by the previously-internal > code in dwfl_set_initial_registers_thread.
You are mostly right, pid_thread_state_registers_cb is basically a convenience wrapper around the already public dwfl_thread_state_register_pc and dwfl_thread_state_registers. But as implemented there are two things that make it an internal (libdwflP.h) interface. It uses a void *arg which really should be a Dwfl_Thread *. If we make it public it should have a normal Dwfl_Thread * (as first) argument. Secondly there is an assert in the call which should be a normal sanity check that return an error when violated if anybody can call it. Also as you already point out it handles a magic, not publicly documented, -2 regnum. We should document it when made a public function. I am not against making it a public libdwfl function with the above changes, but I think it is simpler/less work to just make it an internal libdwflP.h helper function. Cheers, Mark
