Re: [PATCH 5/5] gdb: make find_memory_region_ftype a function_view

[Simon Marchi <simon.marchi@polymtl.ca>]

On 2026-03-13 14:46, Tom Tromey wrote:
>>>>>> "Simon" == simon marchi <simon.marchi@polymtl.ca> writes:
> 
> Simon> From: Simon Marchi <simon.marchi@polymtl.ca>
> Simon> Make it a function_view and remove the `void *` parameter.  Convert the
> Simon> callers (in gcore.c) to use lambda functions that capture `obfd`.
> 
> Simon> One thing is that I am not sure how to implement
> Simon> target_debug_print_find_memory_region_ftype now... right now I made it
> Simon> return a constant "<function_view>" string.
> 
> I think this is fine.  Some of these debug printers are already not
> really that useful.
> 
> Simon> To make target.h see find_memory_region_ftype, I added an include of
> Simon> gdbarch.h in target.h, I'm not sure if this is undesirable, possibly
> Simon> causing some circular include problem in the future.
> Simon> find_memory_region_ftype was previously in defs.h.  Leaving it there
> Simon> would require including gdbsupport/function-view.h inside defs.h, which
> Simon> I would prefer not to do.  An alternative would be to place
> Simon> find_memory_region_ftype in a smaller (perhaps a new) header file, but I
> Simon> don't really know which one.
> 
> It's tempting to try to fix it now before it is a problem.
> 
> Though I somewhat suspect that what you have already is fine.
> 
> I do think having a new small header just for this type would be
> completely fine and I think it's a reasonable long-term strategy for
> handling this kind of "pro forma" typedef.  As in, I'd encourage this in
> other cases where breaking circular dependencies is needed.
> 
> Anyway I also think this is ok.
> Approved-By: Tom Tromey <tom@tromey.com>

Well, it does feel wrong to have the typedef in gdbarch.h, because it is
not more a "gdbarch.h" thing than it is a "target.h"  thing.  Both
headers are equal users of the typedef.  So I think it makes sense for
it to be in a third header that they both include.

I will send a new patch where the typedef is in a new file called
`find-memory-region.h`.

Simon