labath added inline comments.
================ Comment at: source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp:3413 + section->GetName().GetCString()); + return 0; } ---------------- lemo wrote: > labath wrote: > > `lit/Modules/compressed-sections.yaml` test will need to be updated to > > account for the return 0. > compressed-sections.yaml seems to be gated by REQUIRES: zlib, so it would not > hit the 0-length path, am I wrong? The `.bogus` section in that test deliberately contains malformed data, so you should still hit the first error case. However, looking at the lldb-test implementation, it just ignores the `ReadSectionData` return value, and relies on the size in the DataExtractor instead (one day I'll declare war on functions of this type), which is probably why you hadn't noticed this. I guess in this case, it would be nice to reset the data extractor before returning 0 and possibly teaching lldb-test to report discrepancies in the returned sizes. ================ Comment at: source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp:3413 + section->GetName().GetCString(), + llvm::toString(Decompressor.takeError()).c_str()); + return 0; ---------------- This needs to be `std::move(Error)`. If you built with asserts enabled and hit this line, you would crash because you did not consume the `Error` object. https://reviews.llvm.org/D50274 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
