Hi Matthias,
On Mon, Mar 16, 2020 at 12:42:22AM +0100, Matthias Maennich wrote:
> > Just before this hunk we do:
> >
> > if (likely (zrc == Z_OK))
> > zrc = inflateEnd (&z);
> >
> > So, zrc can be !Z_OK because the earlier inflateEnd() failed, which
> > might cause it to call inflateEnd() twice (which might be fine, I
> > dunno). Should we maybe ignore the error if inflateEnd() and just call
> > it unconditionally before (ignoring its return code)?
> >
> > So, replace:
> > if (... Z_OK) zrc = inflateEnd (&z);
> > with unconditionally ending the stream:
> > (void)inflateEnd(&z);
> >
>
> I prefer your variant (and it was my first version of the patch)
> independently from what comes below.
>
> Having said that: I looked up what inflateEnd() does and the worst that
> could happen is returning an error that we anyway ignore. So, duplicate
> calls are not an issue. Also, for the compression part we call
> deflateEnd() via a macro in the same duplicate fashion. Hence I
> consistently used the same pattern for inflateEnd(). And last, I wanted
> to keep that existing error handling. OTOH, projects (including the
> example code of zlib [1]) usually just unconditionally call inflateEnd()
> ignoring any error codes. So, your call :-)
Thanks for looking into this so closely. I think it is best to just do
as the example code does. Always call inflateEnd (), ignoring the
return code and not care whether (in an error case) it might be called
twice. And to make it consistent in this file. So:
diff --git a/libelf/elf_compress.c b/libelf/elf_compress.c
index 244467b5..32422edc 100644
--- a/libelf/elf_compress.c
+++ b/libelf/elf_compress.c
@@ -197,7 +197,7 @@ __libelf_compress (Elf_Scn *scn, size_t hsize, int ei_data,
}
while (flush != Z_FINISH); /* More data blocks. */
- zrc = deflateEnd (&z);
+ deflateEnd (&z);
if (zrc != Z_OK)
{
__libelf_seterrno (ELF_E_COMPRESS_ERROR);
@@ -251,8 +251,7 @@ __libelf_decompress (void *buf_in, size_t size_in, size_t
size_out)
}
zrc = inflateReset (&z);
}
- if (likely (zrc == Z_OK))
- zrc = inflateEnd (&z);
+ inflateEnd (&z);
if (unlikely (zrc != Z_OK) || unlikely (z.avail_out != 0))
{
Thanks,
Mark
