From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gnu.wildebeest.org (gnu.wildebeest.org [45.83.234.184]) by sourceware.org (Postfix) with ESMTPS id 49736383A0F3 for ; Thu, 15 Dec 2022 13:18:03 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 49736383A0F3 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=klomp.org Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=klomp.org Received: from tarox.wildebeest.org (83-87-18-245.cable.dynamic.v4.ziggo.nl [83.87.18.245]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by gnu.wildebeest.org (Postfix) with ESMTPSA id 46C00308B8B5; Thu, 15 Dec 2022 14:17:59 +0100 (CET) Received: by tarox.wildebeest.org (Postfix, from userid 1000) id B34CA40165DE; Thu, 15 Dec 2022 14:17:57 +0100 (CET) Message-ID: Subject: Re: [PATCHv2] support ZSTD compression algorithm From: Mark Wielaard To: Martin =?UTF-8?Q?Li=C5=A1ka?= , elfutils-devel@sourceware.org Date: Thu, 15 Dec 2022 14:17:57 +0100 In-Reply-To: References: <24d7165b-b8ac-ea5d-a046-aec2203696a9@suse.cz> Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Mailer: Evolution 3.28.5 (3.28.5-10.el7) Mime-Version: 1.0 X-Spam-Status: No, score=-3038.7 required=5.0 tests=BAYES_00,GIT_PATCH_0,JMQ_SPF_NEUTRAL,KAM_DMARC_STATUS,SPF_HELO_NONE,SPF_PASS,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: Hi Martin, On Tue, 2022-11-29 at 13:05 +0100, Martin Li=C5=A1ka wrote: > There's second version of the patch that fully support both > compression and decompression. >=20 > Changes from the v1: > - compression support added > - zstd detection is fixed > - new tests are added > - builds fine w/ and w/o the ZSTD library >=20 > What's currently missing and where I need a help: >=20 > 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. >=20 > 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 +=3D 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(-) >=20 > 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/LZM= ALIB/ZSTD .am > dnl conditional and config.h USE_BZLIB/USE_LZMALIB/USE_ZSTD #define. > save_LIBS=3D"$LIBS" > LIBS=3D > +eu_ZIPLIB(zstd,ZSTD,zstd,ZSTD_decompress,[ZSTD (zst)]) > +AS_IF([test "x$with_zstd" =3D xyes], [LIBZSTD=3D"libzstd"], [LIBLZSTD=3D= ""]) > +AC_SUBST([LIBZSTD]) > +zstd_LIBS=3D"$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=3D"$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" =3D xyes], [LIBLZMA=3D"liblzma"], [LIBLZMA=3D= ""]) > AC_SUBST([LIBLZMA]) > -eu_ZIPLIB(zstd,ZSTD,zstd,ZSTD_decompress,[ZSTD (zst)]) > -AS_IF([test "x$with_zstd" =3D xyes], [LIBZSTD=3D"libzstd"], [LIBLZSTD=3D= ""]) > -AC_SUBST([LIBZSTD]) > zip_LIBS=3D"$LIBS" > LIBS=3D"$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 =3D > am_libelf_pic_a_OBJECTS =3D $(libelf_a_SOURCES:.c=3D.os) > =20 > libelf_so_DEPS =3D ../lib/libeu.a > -libelf_so_LDLIBS =3D $(libelf_so_DEPS) -lz > +libelf_so_LDLIBS =3D $(libelf_so_DEPS) -lz $(zstd_LIBS) > if USE_LOCKS > libelf_so_LDLIBS +=3D -lpthread > endif OK. Haven't read the actual code yet. I'll get back to that later today. Cheers, Mark