labath 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: > 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. https://reviews.llvm.org/D39681 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits