List of Build and Runtime Issues

[fegennari]

I'm going to create a single issue for all 5 of these, in the order in which I encountered them when attempting to integrate zstdpp into my project.

1. When building in gcc with -Wall -Werror, I get three unused variable warnings/errors:

In function 'void zstdpp::stream::decompress(std::istream&, std::ostream&, zstdpp::threads_number_t)': zstdpp.hpp:189:22: error: unused variable 'toRead' [-Werror=unused-variable]
In function 'zstdpp::buffer_t zstdpp::compress(const buffer_t&, compress_level_t)': zstdpp.hpp:285:12: error: unused variable 'est_compress_size' [-Werror=unused-variable]
In function 'zstdpp::buffer_t zstdpp::decompress(buffer_t&)': zstdpp.hpp:291:14: error: unused variable 'est_decomp_size' [-Werror=unused-variable]

I suggest removing the line in decompress() and commenting out the return values for the other two, like this if you want to keep them in the code:
/*size_t est_compress_size =*/ inplace::compress(data, comp_buffer);

2. ZSTD_CCtx_setParameter() does not need to be called for nThreads==1. In my case I get the "doesn't support multithreading" error. If you want to keep the error there should be a "quiet" flag to suppress it, or there should be an optional custom ostream or error handler passed in for the user to redirect this to a log file, etc. Right now it will print that error message to stderr on every call.

3. Line 199 confused me at first:
   isEmpty = read == 0;
   I read this as "read = 0". I suggest adding parens:
   isEmpty = (read == 0);

4. There is no error checking on the compress and decompress calls. The return values are always treated as a number of bytes. Any failures due to corrupted streams will return an error code, which is a large integer close to 2^64. Calling something like this:
   out_buffer.resize(decomp_size);out_buffer.resize(decomp_size);
   On the error code will throw std::bad_alloc.

I suggest adding an error check such as:
if( ZSTD_isError(decomp_size) ) { throw read_exception( ZSTD_getErrorName(decomp_size) ); }

5. Reading off the end of the istream when the buffer size is larger than the remaining file size on this line:
   in.read((char*)buffIn.data(), buffInSize);
   will set the error bit on the instream. This causes the read to fail for me. I couldn't find a clean way to handle this. The best solution is probably to get the remaining file size with (total_size - in.tellp()) and read size of min(buffInSize, remaining_size). But this requires more changes to the control flow, for example an initial step to calculate the total file size by seeking to the end.

My hack was to clear the error bit after in.read() in this specific case:
if( in.eof() && in.fail() && !in.bad() ) { in.clear(); }
Maybe you can think of a better solution.