From: Mark Wielaard <mark@klomp.org>
To: "Martin Liška" <mliska@suse.cz>, elfutils-devel@sourceware.org
Subject: Re: [PATCHv2] support ZSTD compression algorithm
Date: Thu, 15 Dec 2022 14:17:57 +0100 [thread overview]
Message-ID: <e0d6b57893f6e070a44e11dbdd0bdad4958dfbb3.camel@klomp.org> (raw)
In-Reply-To: <a0a4205a-2143-53a7-f3ba-6083518cd3d5@suse.cz>
Hi Martin,
On Tue, 2022-11-29 at 13:05 +0100, Martin Liška wrote:
> There's second version of the patch that fully support both
> compression and decompression.
>
> Changes from the v1:
> - compression support added
> - zstd detection is fixed
> - new tests are added
> - builds fine w/ and w/o the ZSTD library
>
> What's currently missing and where I need a help:
>
> 1) When I build ./configure --without-zstd, I don't have
> a reasonable error message (something similar to binutils's readelf:
> readelf: Warning: section '.debug_str' has unsupported compress type:
> 2)
> even though, __libelf_decompress returns NULL and __libelf_seterrno).
> One can see a garbage in the console.
>
> How to handle that properly?
Is there a particular way you are running eu-readelf? Is it with
generic -w or -a, or decoding a specific section type?
> 2) How should I run my newly added tests conditionally if zstd
> configuration support is enabled?
eu_ZIP will define an AM_CONDITIONAL for ZSTD (note this is different
from the HAVE_ZSTD, which is the am conditional added for having the
zstd program itself). You could use it in tests/Makefile.am as:
if ZSTD
TESTS += run-zstd-compress-test.sh
endif
If you had a separate test... Otherwise add some variable to
TESTS_ENVIRONMENT (and installed_TESTS_ENVIRONMENT) based on the Make
variable that is tested inside the shell script (somewhat like
USE_VALGRIND) maybe.
> configure.ac | 8 +-
> libelf/Makefile.am | 2 +-
> libelf/elf_compress.c | 294 ++++++++++++++++++++++++++++++--
> -----
> libelf/elf_compress_gnu.c | 5 +-
> libelf/libelfP.h | 4 +-
> src/elfcompress.c | 144 ++++++++++--------
> src/readelf.c | 18 ++-
> tests/run-compress-test.sh | 24 +++
> 8 files changed, 373 insertions(+), 126 deletions(-)
>
> diff --git a/configure.ac b/configure.ac
> index 59be27ac..07cfa54b 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -410,6 +410,11 @@ dnl Test for bzlib and xz/lzma/zstd, gives BZLIB/LZMALIB/ZSTD .am
> dnl conditional and config.h USE_BZLIB/USE_LZMALIB/USE_ZSTD #define.
> save_LIBS="$LIBS"
> LIBS=
> +eu_ZIPLIB(zstd,ZSTD,zstd,ZSTD_decompress,[ZSTD (zst)])
> +AS_IF([test "x$with_zstd" = xyes], [LIBZSTD="libzstd"], [LIBLZSTD=""])
> +AC_SUBST([LIBZSTD])
> +zstd_LIBS="$LIBS"
> +AC_SUBST([zstd_LIBS])
> eu_ZIPLIB(bzlib,BZLIB,bz2,BZ2_bzdopen,bzip2)
> # We need this since bzip2 doesn't have a pkgconfig file.
> BZ2_LIB="$LIBS"
> @@ -417,9 +422,6 @@ AC_SUBST([BZ2_LIB])
> eu_ZIPLIB(lzma,LZMA,lzma,lzma_auto_decoder,[LZMA (xz)])
> AS_IF([test "x$with_lzma" = xyes], [LIBLZMA="liblzma"], [LIBLZMA=""])
> AC_SUBST([LIBLZMA])
> -eu_ZIPLIB(zstd,ZSTD,zstd,ZSTD_decompress,[ZSTD (zst)])
> -AS_IF([test "x$with_zstd" = xyes], [LIBZSTD="libzstd"], [LIBLZSTD=""])
> -AC_SUBST([LIBZSTD])
> zip_LIBS="$LIBS"
> LIBS="$save_LIBS"
> AC_SUBST([zip_LIBS])
Doing AC_SUBST([zstd_LIBS]) seems correct. Why is the test moved
earlier?
You are testing for ZSTD_decompress, is that enough? Asking because I
see you are using ZSTD_compressStream2, which seems to requires libzstd
v1.4.0+.
In general how stable is the libzstd api?
You'll also need to use the AC_SUBST for LIBZSTD in config/libelf.pc.in
now, as used in the libdw.pc.in:
Requires.private: zlib @LIBZSTD@
> diff --git a/libelf/Makefile.am b/libelf/Makefile.am
> index 560ed45f..24c25cf8 100644
> --- a/libelf/Makefile.am
> +++ b/libelf/Makefile.am
> @@ -106,7 +106,7 @@ libelf_pic_a_SOURCES =
> am_libelf_pic_a_OBJECTS = $(libelf_a_SOURCES:.c=.os)
>
> libelf_so_DEPS = ../lib/libeu.a
> -libelf_so_LDLIBS = $(libelf_so_DEPS) -lz
> +libelf_so_LDLIBS = $(libelf_so_DEPS) -lz $(zstd_LIBS)
> if USE_LOCKS
> libelf_so_LDLIBS += -lpthread
> endif
OK.
Haven't read the actual code yet. I'll get back to that later today.
Cheers,
Mark
next prev parent reply other threads:[~2022-12-15 13:18 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-10-24 11:09 [PATCH][RFC] readelf: partial support of ZSTD compression Martin Liška
2022-10-24 11:41 ` Dmitry V. Levin
2022-10-24 12:17 ` Martin Liška
2022-10-24 16:48 ` Dmitry V. Levin
2022-10-24 18:16 ` Martin Liška
2022-10-28 22:21 ` Mark Wielaard
2022-11-28 13:16 ` Martin Liška
2022-11-28 22:29 ` Mark Wielaard
2022-11-29 9:34 ` Martin Liška
2022-11-29 12:05 ` [PATCHv2] support ZSTD compression algorithm Martin Liška
2022-12-09 10:17 ` Martin Liška
2022-12-15 13:17 ` Mark Wielaard [this message]
2022-12-19 14:19 ` Martin Liška
2022-12-19 15:09 ` Mark Wielaard
2022-12-21 11:13 ` Martin Liška
2023-01-10 17:44 ` [PATCH] readelf: Check compression status of .debug section data Mark Wielaard
2023-01-16 19:39 ` Mark Wielaard
2022-12-16 0:32 ` [PATCHv2] support ZSTD compression algorithm Mark Wielaard
2022-12-19 14:21 ` Martin Liška
2022-12-19 15:16 ` Mark Wielaard
2022-12-21 11:09 ` Martin Liška
2022-12-21 23:14 ` Mark Wielaard
2022-12-22 9:17 ` Martin Liška
2022-12-22 18:36 ` Mark Wielaard
2022-12-23 8:44 ` Martin Liška
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=e0d6b57893f6e070a44e11dbdd0bdad4958dfbb3.camel@klomp.org \
--to=mark@klomp.org \
--cc=elfutils-devel@sourceware.org \
--cc=mliska@suse.cz \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).