[Lldb-commits] [PATCH] D59775: Minidump: Add support for reading/writing strings
Pavel Labath via Phabricator via lldb-commits
lldb-commits at lists.llvm.org
Thu Apr 4 06:47:01 PDT 2019
labath marked 3 inline comments as done.
labath added inline comments.
Comment at: include/llvm/ObjectYAML/MinidumpYAML.h:70
+ std::string CSDVersion;
> Don't know about lifetime here, but could this be a `StringRef`?
A StringRef would work when constructing this object from YAML (as there the yaml file can provide backing storage). However, when constructing this from a Minidump binary, that would not work, as the data is stored there in the utf16 form (so not the kind of "string" we usually work with).
It would probably be possible to pull of some kind of a trick like `yaml::BinaryRef` does, where this would be a special object, which could be backed either by a real (utf8) string, or by the minidump utf16 thingy. However, I don't think that would be worth the trouble, as these strings are fairly small. (Also, in order to validate the utf16 data (so I don't end up creating an invalid object and crash during serialization), I'd have to essentially decode the data into a std::string anyway and then throw the result away.)
Comment at: unittests/Object/MinidumpTest.cpp:270
+ 0, // Mis-align next string
+ 2, 0, 0, 0, 'a', 0, // String6 - "a"
> What about a case where the size field cannot fit in the remaining data?
Ah yes, good catch. I'll add that.
CHANGES SINCE LAST ACTION
More information about the lldb-commits