[Lldb-commits] [PATCH] D60121: Object/Minidump: Add support for reading the ModuleList stream

[Pavel Labath via Phabricator via lldb-commits]

labath marked an inline comment as done.
labath added inline comments.


================
Comment at: lib/Object/Minidump.cpp:55
+  if (!OptionalStream)
+    return createError("No such stream", object_error::invalid_section_index);
+  auto ExpectedSize =
----------------
labath wrote:
> jhenderson wrote:
> > I wonder whether it would be worth a new class of errors for minidump files? After all, invalid_section_index feels a bit forced for a format without sections!
> Yes, I've been wondering about that too. In practice, I don't expect that the consumers will wan't to differentiate the error cases too much here (it usually does not matter if the stream was not present in the file, or if it was truncated -- the end result is the same -- you cannot use it). However, for clarity it might be better to do have a separate type anyway. I'll whip up a separate patch for that.
After some soul-searching I decided to just abandon the idea of returning error codes here, and just return the generic parse_failed error. That's what most other Object formats do (presumably they don't have a good use for distinguishing the codes either), and having an own error type would require some non-obvious design choices. (Like, since I'd still inherit from BinaryError, which inherits from ECError, I'd still have to interoperate with std::error_code somehow. This means I'd still have to map things to the object_error enum, or I'd have to define a new error_category, which is a deprecated way of doing things).


Repository:
  rL LLVM

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

https://reviews.llvm.org/D60121