[Lldb-commits] [PATCH] D41725: [lldb] [test] Fix tests to use more portable LLVM_ENABLE_ZLIB

Jason Molenda via llvm-commits llvm-commits at lists.llvm.org
Tue Jan 9 11:36:21 PST 2018



> On Jan 9, 2018, at 6:51 AM, Michał Górny via Phabricator via lldb-commits <lldb-commits at lists.llvm.org> wrote:
> 
> mgorny added a comment.
> 
> In https://reviews.llvm.org/D41725#970926, @labath wrote:
> 
>> In https://reviews.llvm.org/D41725#969388, @mgorny wrote:
>> 
>>> I was talking of:
>>> 
>>>  source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
>>>  source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
>> 
>> 
>> Ah, right. These uses were introduced before we developed a preference for llvm primitives. That said, I see a lot of compression stuff there, but I don't see mention of zlib directly.
> 
> 
> In the first of those files there are two `HAVE_LIBZ` conditionals that use zlib. There's also `HAVE_LIBCOMPRESSION` for the other library, I suppose it's relevant to Darwin.
> 
> Therefore, I have three questions:
> 
> 1. Should we add a separate check/switch for zlib here, or just use `LLVM_ENABLE_ZLIB`?
> 2. Should I post a patch rewriting this little zlib bit to reuse libLLVMSupport?
> 3. Should I look into merging libcompression support into libLLVMSupport?



For lldb main sources, it's fine to use llvm's zlib impl instead of libz.  In the debugserver project under tools/, we still link to libz because debugserver does not have any dependencies on llvm.

I haven't looked at the llvm compression API so I don't know how easy it would be to add libcompression support.  libcompression is only available on macOS 10.11 ("El Capitan", released late 2015) and newer.  The API is documented here:

https://developer.apple.com/documentation/compression/data_compression?language=objc

on Darwin systems, lldb will prefer the lzfse compression algorithm when both sides are using libcompression.  It will fall back to zlib-deflate as its next choice.



J



More information about the llvm-commits mailing list