Skip to content

WIP Replace ASSERT by real check#723

Draft
mgautierfr wants to merge 1 commit into
mainfrom
error_check_assert
Draft

WIP Replace ASSERT by real check#723
mgautierfr wants to merge 1 commit into
mainfrom
error_check_assert

Conversation

@mgautierfr

@mgautierfr mgautierfr commented Aug 11, 2022

Copy link
Copy Markdown
Contributor

Fixes #670

@kelson42

kelson42 commented Mar 8, 2023

Copy link
Copy Markdown
Contributor

@mgautierfr This PR is alsmot 8 months old as it was pretty time consuming to finish it. I had two days of work in my mind. Is that correct?

@mgautierfr

Copy link
Copy Markdown
Contributor Author

At least yes.
I have put a lot of check in the low level reader using assert but we want here remove the assert (or keep it in debug, but in release is it as remove it) and add check at higher level.
This means passing through all places where data (coming from the zim file itself) may be wrong and raise a explicit exception "as soon as possible". But those places are not identified, we will probably have a lot of round trip during the review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

The reader test seems to stuck in a deadlock (zim-testing-suite) for some reason.

2 participants