[PATCH] D78558: [Support] Make DataExtractor error messages more clear

[Pavel Labath via Phabricator via llvm-commits]

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


================
Comment at: llvm/lib/Support/DataExtractor.cpp:25
+      *E = createStringError(
+          errc::illegal_byte_sequence,
+          "unexpected end of data at offset 0x%zx while reading [0x%" PRIx64
----------------
jhenderson wrote:
> labath wrote:
> > jhenderson wrote:
> > > FWIW, I'm not convinced `illegal_byte_sequence` is really the right thing to use. I had a discussion with someone else recently about this, but in every example usage I found online outside LLVM, it is for unicode encoding issues. The standard is silent on the exact purpose of it so the argument is that it's perfectly reasonable to use it for other bad sequences. I personally would use `invalid_argument` here, since you're trying to read using an invalid `Offset`.
> > Yes, invalid_argument looks better here.
> Looks like you only changed the second case here? Potentially, `Size` could be invalid causing the error. What do you think?
Yes, that was intentional. I don't think we should blame the Size variable here. Since that is a long lived member variable, I think we should be assuming that it is correct. The first thing to blame should be the Offset argument, but in this case that also looks plausible, so we blame the data contents. For that `illegal_byte_sequence` seems appropriate (with the unicode caveat).

I think this is similar to how e.g. passing a bogus pointer to a system call results in EFAULT, but passing a pointer to incorrect data results in a more specific error code.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78558