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
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
