bsdjhb added inline comments.

================
Comment at: source/Plugins/Process/elf-core/elf-core-enums.h:14
+/// Core files PT_NOTE segment descriptor types
+enum {
+  NT_PRSTATUS = 1,
----------------
krytarowski wrote:
> labath wrote:
> > krytarowski wrote:
> > > alexandreyy wrote:
> > > > krytarowski wrote:
> > > > > alexandreyy wrote:
> > > > > > krytarowski wrote:
> > > > > > > No namespace here?
> > > > > > I think these constants are used for multiple OSs.
> > > > > > Am I correct?
> > > > > > Do you have a suggestion for a namespace?
> > > > > There is rumor that they came from SVR4.. but I don't have the specs 
> > > > > to make sure.
> > > > > 
> > > > > Multiple in this case does not mean "all". This is not true for at 
> > > > > least NetBSD.
> > > > I can remove these constants and modify the first "if" in 
> > > > ParseThreadContextsFromNoteSegment to:
> > > > 
> > > >   if (((note.n_type == LINUX::NT_PRSTATUS ||
> > > >           note.n_type == FREEBSD::NT_PRSTATUS) && have_prstatus) ||
> > > >         ((note.n_type == LINUX::NT_PRPSINFO ||
> > > >           note.n_type == FREEBSD::NT_PRPSINFO) && have_prpsinfo)) {
> > > >       assert(thread_data->gpregset.GetByteSize() > 0);
> > > > 
> > > > What do you think?
> > > > But NT_PRSTATUS and NT_PRPSINFO have the same value for Linux and 
> > > > FreeBSD .
> > > I propose to put NT_PRSTATUS, NT_PRFPREG, NT_PRPSINFO into this namespace 
> > > and call it SVR4.
> > > 
> > > SmartOS uses the same values.
> > With the addition of all the bsd variants, the note parsing code has grown 
> > pretty big.
> > 
> > My plan was to refactor it a bit after this is committed. I wanted to split 
> > it into two stages: first we just determine the OS from the notes; and then 
> > we dispatch to an appropriate os-specific function to do the actual parsing 
> > (with each function just using the NT constants from its own namespace). If 
> > you agree with that direction then I propose to go with the 
> > LINUX::NT_PRSTATUS || FREEBSD::NT_PRSTATUS proposed by alexandreyy in the 
> > interim.
> Sounds good.
Yes, that is probably the right approach.  In binutils the note parsing code 
switches on the note name ("FreeBSD" vs "NetBSD" vs "CORE" vs "Linux", etc.) 
and then inside name-specific functions switches on the note type.  I've been 
adding support for more notes to GDB for FreeBSD and ended up splitting out a 
handler for "FreeBSD" notes inline with this approach.  Even notes with the 
same type (NT_PRSTATUS, etc.) have different layouts across OS's.


Repository:
  rL LLVM

https://reviews.llvm.org/D39681



_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to