This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: RFA: Try to include libunwind-ia64.h in libunwind-frame.h


On 02/14/2012 02:35 PM, Jan Kratochvil wrote:
> At least `libunwind_frame_unwind' is dead - it has no references in the
> codebase - which confused me.

It's dead for IA-64 as well.  It's likely dead for any other arch that may
want to use libunwind.  So "if the non-ia64 libunwind support is therefore
really removed the dead code in libunwind-frame.c should be also removed"
is not the right justification to remove the dead code.  If you want to
remove it, go ahead, but do it because it's dead, period.

> Also when you change GDB design by this patch - from arch-independent
> libunwind-frame.c to ia64-limited libunwind-frame.c - one should best rename
> libunwind-frame.[ch] to libunwind-ia64-frame.[ch].  Otherwise at least write
> some comments there this file is used only for ia64 targets now.

I have already added such comments to libunwind-frame.h.  The limitation that
the file is only usable by ia-64 is pre-existing.  I'm not adding anything
new, other than making the limitation _explicit_, which is a good thing.

I'm adding this comment to libunwind-frame.c

/* IA-64 is the only target that currently uses libunwind-frame.  Note
   how UNW_TARGET, UNW_OBJ, etc. are compile time constants below.
   Those come from libunwind's headers, and are target dependent.
   Also, some of libunwind's typedefs are target dependent, as e.g.,
   unw_word_t.  If some other target wants to use this, we will need
   to do some abstracting in order to make it possible to select which
   libunwind we're talking to at runtime (and have one per arch).  */

Are you fine with that?

>> > The code is not really ia64 specific.
> Yes, because it was designed to be possibly used in the future with arbirary
> arch.

I don't understand what are we discussing.  The possibility is there, but it
needs work to get there.  When the future comes, we'll have to adjust.  Right
now, nobody but IA-64 cares.  Making the limitation explicit by including
the ia64 header directly doesn't make the needed work more difficult one
single bit.  On the contrary.

-- 
Pedro Alves


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]