Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP Replace ASSERT by real check #723

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion meson.build
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
project('libzim', ['c', 'cpp'],
version : '7.2.2',
license : 'GPL2',
default_options : ['c_std=c11', 'cpp_std=c++11'])
default_options : ['c_std=c11', 'cpp_std=c++11', 'b_ndebug=if-release'])

if build_machine.system() != 'windows'
add_project_arguments('-D_LARGEFILE64_SOURCE=1', '-D_FILE_OFFSET_BITS=64', language: 'cpp')
Expand Down
5 changes: 4 additions & 1 deletion src/cluster.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,9 @@ getClusterReader(const Reader& zimReader, offset_t offset, Cluster::Compression*
while (--n_offset)
{
OFFSET_TYPE new_offset = seqReader.read<OFFSET_TYPE>();
ASSERT(new_offset, >=, offset);
if (new_offset < offset) {
throw ZimFileFormatError("Offsets in cluster must be increasing.")
}

m_blobOffsets.push_back(offset_t(new_offset));
offset = new_offset;
Expand All @@ -142,6 +144,7 @@ getClusterReader(const Reader& zimReader, offset_t offset, Cluster::Compression*
auto blobSize = getBlobSize(blob_index_t(current));
if (blobSize.v > SIZE_MAX) {
m_blobReaders.push_back(std::unique_ptr<Reader>(new BufferReader(Buffer::makeBuffer(zsize_t(0)))));
m_reader->skip(blobSize.v);
} else {
m_blobReaders.push_back(m_reader->sub_reader(blobSize));
}
Expand Down
2 changes: 1 addition & 1 deletion src/debug.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
#include <stdexcept>
#include <stdlib.h>

#if defined (NDEBUG)
#if 1
# define ASSERT(left, operator, right) (void(0))
#else

Expand Down
3 changes: 3 additions & 0 deletions src/file_reader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,9 @@ MultiPartFileReader::MultiPartFileReader(std::shared_ptr<const FileCompound> sou

char MultiPartFileReader::read(offset_t offset) const {
ASSERT(offset.v, <, _size.v);
if (offset.v >= _size.v) {
throw std::runtime_error("Error while reading out of reader");
}
offset += _offset;
auto part_pair = source->locate(offset);
auto& fhandle = part_pair->second->fhandle();
Expand Down
5 changes: 4 additions & 1 deletion src/fs_unix.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,10 @@ zsize_t FD::readAt(char* dest, zsize_t size, offset_t offset) const
while (size_to_read > 0) {
auto size_read = PREAD(m_fd, dest, size_to_read, current_offset);
if (size_read == -1) {
return zsize_t(-1);
throw std::runtime_error("Error while reading.");
}
if (size_read == 0) {
throw std::runtime_error("Error reading after the end of file.");
}
size_to_read -= size_read;
current_offset += size_read;
Expand Down
3 changes: 3 additions & 0 deletions src/istreamreader.h
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,9 @@ class IStreamReader
// Reads a blob of the specified size from the stream
virtual std::unique_ptr<const Reader> sub_reader(zsize_t size);

// Skip a amount of bytes in the stream
virtual void skip(zsize_t size);

private: // virtual methods
// Reads exactly 'nbytes' bytes into the provided buffer 'buf'
// (which must be at least that big). Throws an exception if
Expand Down
8 changes: 7 additions & 1 deletion src/writer/creator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -396,6 +396,7 @@ namespace zim
clusterSize(clusterSize),
withIndex(withIndex),
indexingLanguage(language),
mimeTypeLeftSpace(CLUSTER_BASE_OFFSET-80),
verbose(verbose),
nbRedirectItems(0),
nbCompItems(0),
Expand Down Expand Up @@ -660,8 +661,13 @@ namespace zim
auto it = mimeTypesMap.find(mimeType);
if (it == mimeTypesMap.end())
{
if (nextMimeIdx >= std::numeric_limits<uint16_t>::max())
if (nextMimeIdx >= std::numeric_limits<uint16_t>::max()) {
throw std::runtime_error("too many distinct mime types");
}
if (mimeType.size() >= mimeTypeLeftSpace) {
throw std::runtime_error("Too many mime types for the space we have allocated");
}
mimeTypeLeftSpace -= mimeType.size() - 1;
mimeTypesMap[mimeType] = nextMimeIdx;
rmimeTypesMap[nextMimeIdx] = mimeType;
return nextMimeIdx++;
Expand Down
4 changes: 3 additions & 1 deletion test/reader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,9 @@ TEST(FileReader, shouldJustWork)
ASSERT_THROW(reader->read(offset_t(26)), std::runtime_error);
ASSERT_THROW(reader->read(out, offset_t(25), zsize_t(4)), std::runtime_error);
ASSERT_THROW(reader->read(out, offset_t(30), zsize_t(4)), std::runtime_error);
ASSERT_THROW(reader->read(out, offset_t(30), zsize_t(0)), std::runtime_error);
// Reading 0 bytes is a no op. The (debug) ASSERT is catching the wrong offset first,
// but in release, without assert, this is really a noop and no error must be checked.
//ASSERT_THROW(reader->read(out, offset_t(30), zsize_t(0)), std::runtime_error);
}
}

Expand Down