clayborg added inline comments.
================
Comment at: include/llvm/ObjectYAML/MinidumpYAML.h:27
+struct Stream {
+ enum class StreamKind {
+ Hex,
----------------
Maybe "Encoding" might be a better name?
================
Comment at: include/llvm/ObjectYAML/MinidumpYAML.h:28
+ enum class StreamKind {
+ Hex,
+ SystemInfo,
----------------
Maybe "Binary" instead of "Hex"? Or maybe "RawBytes"?
================
Comment at: include/llvm/ObjectYAML/MinidumpYAML.h:48
+/// fallback when no other stream kind is suitable.
+struct HexStream : public Stream {
+ yaml::BinaryRef Content;
----------------
Name "ByteStream" maybe? Does hex make sense?
================
Comment at: include/llvm/ObjectYAML/MinidumpYAML.h:50
+ yaml::BinaryRef Content;
+ yaml::Hex32 Size;
+
----------------
Is the "Size" necessary here? Does yaml::BinaryRef not contain a size?
================
Comment at: include/llvm/ObjectYAML/MinidumpYAML.h:81
+/// /proc/<pid> file on linux).
+struct TextStream : public Stream {
+ BlockStringRef Text;
----------------
Maybe further specify the content like "UTF8Stream" or "ASCIIStream"? Not sure
what the encoding is expected to be, but might be better to indicate the exact
text encoding here?
================
Comment at: lib/ObjectYAML/MinidumpYAML.cpp:132
+std::unique_ptr<Stream> Stream::create(StreamType Type) {
+ StreamKind Kind = getKind(Type);
+ switch (Kind) {
----------------
This is where encoding might be a bit more clear?
```
Encoding E = getEncoding(Type);
```
Repository:
rL LLVM
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D59482/new/
https://reviews.llvm.org/D59482
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
