request for review: lbzip2-0.17-1
On Thu, 29 Oct 2009, Paul Wise wrote:
[apologies for the lateness of this reply]
Still too early :), I just noticed I'll have to fix the debian-sanity
target, because in lbzip2-0.16rc1, the set of recognized environment
variables changed.
Mainly I was concerned about the "may be used uninitialized in this
function" warnings. I guess those are invalid though?
Why thank you, I'm pleased to announce lbzip2-0.17. See below.
main.c:709: warning: 'save_errno' may be used uninitialized in this function
This is invalid, see (0 == ret).
lbunzip2.c:1476: warning: 'reord_needed.last_decompr' may be used unnitialized in this function
lbunzip2.c:1402: note: 'reord_needed.last_decompr' was declared here
lbunzip2.c:1476: warning: 'reord_needed.w2w_blk_id.last_bzip2' may be used uninitialized in this function
lbunzip2.c:1402: note: 'reord_needed.w2w_blk_id.last_bzip2' was declared here
This is a portability bug in lbzip2, thank you for finding it. The gcc
version in lenny didn't report these warnings. The offending fields are
never accessed explicitly, so there is no harm on a "friendly" platform.
However, strictly adhereing to the C standard, the assignment on line 1476
might access a trap representation and lead to undefined behavior. I was
able to fix this bug by creating a dedicated structure without the
offending fields. I've put your name in the README and the manual.
lbunzip2.c:582: warning: 'save_bitbuf' may be used uninitialized in this function
This is invalid, see (0u < ub).
lbunzip2.c:1071: warning: 'ibitbuf' may be used uninitialized in this function
lbunzip2.c:1071: note: 'ibitbuf' was declared here
This is invalid, see "ibits_left = 0u" and (0 == ibits_left).
lbunzip2.c:1083: warning: 'rbitbuf' may be used uninitialized in this function
lbunzip2.c:1083: note: 'rbitbuf' was declared here
This is invalid, see the (IST_IN_BZIP2 == istate) around the rbitbuf read
accesses and the only "istate = IST_IN_BZIP2" assignment following
"rbitbuf = 0u".
lacos_rbtree.c:157: warning: 'cmp_res' may be used uninitialized in this function
This is invalid. The first read access occurs in the loop, but to get
there, an assignment has to happen first in the loop test. The second
access depends on "parent" being non-NULL. "parent" starts out NULL; the
only place where it can change to non-NULL is the loop body, but to get
there... see first case.
BTW, I just ran the test suite and got this message a few times from
p7zip (4.58):
Error:
Reading archives from stdin is not implemented
Command exited with non-zero status 7
This is expected and handled; test.sh allows tested compressors to refuse
test cases.
1. lintian --info --display-info --display-experimental --pedantic
--show-overrides --checksums --color auto
That's useful, thanks. Now that I executed lintian like this, it was
silent.
http://mentors.debian.net/debian/pool/main/l/lbzip2
Changes:
lbzip2 (0.17-1) unstable; urgency=low
.
* New upstream release fixes possible undefined behavior, reported by Paul
Wise in http://lists.debian.org/debian-mentors/2009/10/msg00470.html.
* Adapt the "debian-sanity" target to the new environment variables
introduced by lbzip2-0.16rc1.
* Following http://lists.debian.org/debian-mentors/2009/10/msg00470.html:
- Add -Wall to CFLAGS.
- Add descriptive headers to the makefile and manual page patches.
Please review it :)
Thank you very much for helping newbies (me among them) with such
endurance and responsiveness!
Cheers,
lacos
deb-0_17-1-try-01
Reply to: