[Lldb-commits] [PATCH] D32149: Correct handling NetBSD core(5) files with threads

[Kamil Rytarowski via lldb-commits]

Thanks, I will give it a try!

On 27.04.2017 17:59, Zachary Turner wrote:
> In case it's not obvious, note the space in the command I said to run.
>  `git-clang-format` has a dash, and when you run `git clang-format`
> (with a space), it will run the file with the dash.
> 
> On Thu, Apr 27, 2017 at 8:58 AM Zachary Turner <zturner at google.com
> <mailto:zturner at google.com>> wrote:
> 
>     There is a file in the repo called git-clang-format.  Make sure that
>     file is on your PATH somewhere, then just run `git clang-format`. 
>     It will only touch lines that are part of your diff, and leave
>     surrounding lines alone.  When making a diff, we only want to
>     clang-format the lines we touched, not the entire files.
> 
>     On Thu, Apr 27, 2017 at 8:56 AM Kamil Rytarowski via Phabricator
>     <reviews at reviews.llvm.org <mailto:reviews at reviews.llvm.org>> wrote:
> 
>         krytarowski added inline comments.
> 
> 
>         ================
>         Comment at: source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp:302
>         +  // default 32 or 64 bit arch (without any architecture
>         revision) based on
>         +  // object file's class.
>            if (header.e_type == ET_CORE) {
>         ----------------
>         joerg wrote:
>         > Unrelated cosmetic change.
>         I let clang-format to go and alter minor things. I can run
>         clang-format over original files - commit, and add my diff again.
> 
> 
>         ================
>         Comment at: source/Plugins/Process/elf-core/ProcessElfCore.cpp:463
> 
>         -enum { NT_PROCINFO = 1, NT_AUXV, NT_AMD64_REGS = 33,
>         NT_AMD64_FPREGS = 35 };
>         +enum { NT_PROCINFO = 1, NT_PROCINFO_SIZE = 160, NT_AUXV = 2 };
>         +
>         ----------------
>         joerg wrote:
>         > Either sort them by value or by name, but not randomly
>         I will split this enum{} into two enums.
> 
> 
>         ================
>         Comment at: source/Plugins/Process/elf-core/ProcessElfCore.cpp:523
>         +
>         +  offset += 108;
>         +  cpi_nlwps = data.GetU32(&offset); /* number of LWPs */
>         ----------------
>         joerg wrote:
>         > Can you define a constant for the offset here below instead of
>         a magic number?
>         I will try to get something to define aliases for these magic
>         numbers.
> 
> 
>         ================
>         Comment at: source/Plugins/Process/elf-core/ProcessElfCore.cpp:844
>         +  if (m_thread_data.size() != nlwps)
>         +    return Error("rror parsing NetBSD core(5) notes: Mismatch
>         between the "
>         +                 "number of LWPs in netbsd_elfcore_procinfo and
>         the number of "
>         ----------------
>         joerg wrote:
>         > Typo
>         OK
> 
> 
>         ================
>         Comment at: source/Plugins/Process/elf-core/ProcessElfCore.cpp:854
>         +  /* Signal destinated for a particular LWP */
>         +  else {
>         +    bool passed = false;
>         ----------------
>         joerg wrote:
>         > Move the else to the } and the comment after the {
>         OK
> 
> 
>         Repository:
>           rL LLVM
> 
>         https://reviews.llvm.org/D32149
> 
> 
> 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: OpenPGP digital signature
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20170427/34bca8e8/attachment.sig>