This patch helps change the return type of writeVariableSizedInteger() from void to Error.
I think the error message "invalid integer write size" makes little sense here. If multiple sections are involved, it's hard to tell which section contains the invalid value. I'm still thinking about improving it.
This probably wants to be not_supported because strictly, there's nothing stopping us writing a 3/5/7 etc byte value.
Perhaps here and at similar points, it might be worth adding additional context to the message, to address your problem. There are examples of this in various places, such as llvm-readobj, I believe.
perhaps entry -> list
Similarly, entry -> list here and below.
Thanks for reviewing!
I added a helper function errorWithContext() which helps add additional context information to the error message.
I'm not sure if I'm doing it in the right way. I added one helper function errorWithContext() which helps add additional information to the error message.
This isn't too far off what I had in mind, but is probably more than what you actually need.
Take a look at ELFDumper.cpp in llvm-readobj. You'll see several instances where the function toString is used to convert an error into a string and then this string is used as part of a new error message. For example, in printRelocation you have the following:
Expected<std::pair<const typename ELFT::Sym *, std::string>> Target = this->dumper()->getRelocationTarget(SymTab, R); if (!Target) this->reportUniqueWarning(createError( "unable to print relocation " + Twine(RelIndex) + " in section " + Twine(SecIndex) + ": " + toString(Target.takeError())));
When the Target contains an error, the toString method is used, combined with other context information into a new error message. Consequently, you'd explicitly build up your message with the lower-level error message and higher level things. For example:
if (Error Err = writeVariableSizedInteger( Descriptor.Address, Range.AddrSize, OS, DI.IsLittleEndian)) return createStringError(errc::not_supported, "unable to write debug_aranges address: " + toString(std::move(Err)));
I'd slightly modify this error to include the size that has been requested.
createStringError follows printf style formatting rules, so this can be:
return createStringError(errc::not_supported, "invalid integer write size: %zu", Size);
and then no need to include the header.
Similarly, consider using %s:
return createStringError(errc::not_supported, "unable to write debug_aranges address: %s", toString(std::move(Err)).c_str());
Same goes below.