[llvm] r261298 - llvm-dwp: Don't test compression when zlib isn't available

[Davide Italiano via llvm-commits]

On Thu, Feb 18, 2016 at 6:03 PM, David Blaikie via llvm-commits
<llvm-commits at lists.llvm.org> wrote:
> Author: dblaikie
> Date: Thu Feb 18 20:03:45 2016
> New Revision: 261298
>
> URL: http://llvm.org/viewvc/llvm-project?rev=261298&view=rev
> Log:
> llvm-dwp: Don't test compression when zlib isn't available
>
> Modified:
>     llvm/trunk/test/tools/llvm-dwp/X86/compress.test
>     llvm/trunk/tools/llvm-dwp/llvm-dwp.cpp
>
> Modified: llvm/trunk/test/tools/llvm-dwp/X86/compress.test
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/tools/llvm-dwp/X86/compress.test?rev=261298&r1=261297&r2=261298&view=diff
> ==============================================================================
> --- llvm/trunk/test/tools/llvm-dwp/X86/compress.test (original)
> +++ llvm/trunk/test/tools/llvm-dwp/X86/compress.test Thu Feb 18 20:03:45 2016
> @@ -1,6 +1,8 @@
>  RUN: llvm-dwp %p/../Inputs/compress/a.dwo -o %t
>  RUN: llvm-dwarfdump %t | FileCheck %s
>
> +REQUIRES: zlib
> +
>  Simple test built from this input which produces DWARF long enough to be compressed in the .[z]debug_info section:
>
>    void f(int a, int b, int c, int d) {
>
> Modified: llvm/trunk/tools/llvm-dwp/llvm-dwp.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-dwp/llvm-dwp.cpp?rev=261298&r1=261297&r2=261298&view=diff
> ==============================================================================
> --- llvm/trunk/tools/llvm-dwp/llvm-dwp.cpp (original)
> +++ llvm/trunk/tools/llvm-dwp/llvm-dwp.cpp Thu Feb 18 20:03:45 2016
> @@ -350,7 +350,7 @@ static std::error_code write(MCStreamer
>          uint64_t OriginalSize;
>          if (!zlib::isAvailable() ||
>              !consumeCompressedDebugSectionHeader(Contents, OriginalSize))
> -          continue;
> +          return make_error_code(std::errc::invalid_argument);
>          UncompressedSections.resize(UncompressedSections.size() + 1);
>          if (zlib::uncompress(Contents, UncompressedSections.back(), OriginalSize) !=
>              zlib::StatusOK) {
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits

I picked one random llvm-dwp commit to make a more general comment
(please bear with me).
Please consider those as suggestions, and feel free to disagree with me.

I've been trying to make over the last months the error handling in
the tools a little bit more uniform (Rafael did some earlier work on
this area too).
Almost each tool I touched calls exit(1) on error, whilst I noticed
you just return an error code to the caller. Is there a particular
reason why you're doing this? From what I see all the error conditions
you're running into are fatal (at least this is what I gathered from a
quick scan of the code).

Also, writeStringsAndOffsets() returns std::error_code() even if it
never fails for now.
I assumed you put that "just in case" but in other tools (most
noticeably lld) we decided to change return type to std::error to void
until we don't have a case where the function can fail for real.

Thanks,

-- 
Davide

"There are no solved problems; there are only problems that are more
or less solved" -- Henri Poincare