Re: [PATCH v3 1/2] gdb: dwarf2 generic implementation for caching function data

[Tom Tromey <tom@tromey.com>]

>>>>> Torbjörn SVENSSON via Gdb-patches <gdb-patches@sourceware.org> writes:

> v2 -> v3:
> Addressed comments from Tom in
> https://sourceware.org/pipermail/gdb-patches/2023-January/195882.html

Sorry, I'm afraid you missed a few.

> +struct dwarf2_frame_fn_data
> +{
> +  /* The cookie to identify the custom function data by.  */
> +  fn_prev_register cookie;

The previous review mentioned changing the type of this, but honestly I
don't really care about that one, this is as good as anything now that
it's documented.

> +/* See frame.h.  */
> +
> +void *dwarf2_frame_get_fn_data (frame_info_ptr this_frame, void **this_cache,
> +				fn_prev_register cookie)

gdb style puts the function name at the start of the line.
There are many examples in the source.

> +void *dwarf2_frame_allocate_fn_data (frame_info_ptr this_frame,
> +				     void **this_cache,
> +				     fn_prev_register cookie,
> +				     unsigned long size)

Here too.

> +{
> +  struct dwarf2_frame_fn_data *fn_data = nullptr;
> +  struct dwarf2_frame_cache *cache
> +    = dwarf2_frame_cache (this_frame, this_cache);
> +
> +  /* First try to find an existing object.  */
> +  void *data = dwarf2_frame_get_fn_data (this_frame, this_cache, cookie);
> +  if (data)
> +    return data;

It seems to me that there is no need to do this check.
IMO it's fine to just assert that it isn't found.

thanks,
Tom