[PATCH] D61423: MinidumpYAML: add support for the ThreadList stream

Thu May 9 08:11:03 PDT 2019

labath marked 6 inline comments as done.
labath added a comment.

Thanks for the review.

================
Comment at: test/tools/obj2yaml/basic-minidump.yaml:47-49
+      - Thread Id:       0x5C5D5E5F
+        Priority Class:  0x60616263
+        Environment Block: 0x6465666768696A6B
----------------
jhenderson wrote:
> labath wrote:
> > jhenderson wrote:
> > > It would be nice if these were padded so that they all line up. Ditto in the Stack block below.
> > The microsoft structure definition calls this field just "teb" (for Thread Environment Block), but I've found that too opaque, so I expanded the acronym (sans "thread", because it is obvious we are talking about threads here). I could shorten this further to "environment" (the word "block" probably doesn't add that much value) , or even to "teb" for consistency with microsoft headers. Let me know what you think.
> Environment Block is fine. I was actually referring to the number of spaces between the attribute name and value, i.e. I'd prefer this:
> 
> ```
>       - Thread Id:         0x5C5D5E5F
>         Priority Class:    0x60616263
>         Environment Block: 0x6465666768696A6B
> ```
Ok, I see. I can do that manually here, but that won't prevent the actual output from obj2yaml from being misaligned (which is why i was trying to come up with a shorter name).

Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61423/new/

https://reviews.llvm.org/D61423