[Bug 166515] Review Request: pbzip2 : parallel version of bzip2

[bugzilla at redhat.com]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.

Summary: Review Request: pbzip2 : parallel version of bzip2


https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=166515





------- Additional Comments From jnovy at redhat.com  2005-08-25 13:34 EST -------
>From a brief look into the code:

<nitpick>
bzerr_dummy variable is unused in pbzip2.cpp:1191
</nitpick>

Another thing in pbzip2.cpp:

ret = read(hInfile, tmpBuff, strlen(bz2Header)+1);
close(hInfile);
if ((ret == -1) || (ret < strlen(bz2Header)+1))

Note that 'ret' variable is int but should be of size_t type so I'd use
some other than 'ret' variable here: (bzip2.c:1651-1654)

size_t size;
...
size = read(hInfile, tmpBuff, strlen(bz2Header)+1);
close(hInfile);
if ((size == (size_t)(-1)) || (size < strlen(bz2Header)+1))

numBlocks in pbzip2.cpp:1800 should be better of off_t data type.
BTW. why do you define uint_special and don't let it off_t at all?

Suggest getting rid of the redundant modulo and condition:

if ((fileSize % blockSize) == 0)
        numBlocks = fileSize / blockSize;
else
        numBlocks = fileSize / blockSize + 1;

by:
numBlocks = (fileSize + blockSize - 1) / blockSize;

Note that you don't previously check that blockSize != 0,
So that pbzip2 -b0 <anyfile> ends up with Floating point exception!!

Needs further fixes. I'll send you a patch to fix these and other issues I found.

-- 
Configure bugmail: https://bugzilla.redhat.com/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.