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

Fix possible UB when accessing empty vector's data #3481

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

nocun
Copy link
Contributor

@nocun nocun commented Jun 30, 2021

Make sure that the empty vector's data is never used, at() will throw an exception in that case.

This may allow compilers to better optimize code.

@stweil
Copy link
Contributor

stweil commented Jun 30, 2021

I'd expect that code using at() is slower, so I am not sure whether this is really an improvement. Did you test code size and performance?

@nocun
Copy link
Contributor Author

nocun commented Jun 30, 2021

I did not perform large scale tests so can't be sure if this is faster in all cases.

In my test case (single long line) this looks to be faster. Init() is not called that many times (about 30) and there should be virtually no exception related performance hit cause nothing is thrown.

@nocun
Copy link
Contributor Author

nocun commented Jun 30, 2021

On the other hand it may be beneficial to do some testing on non-x86 platforms but I do not have those available atm.

Edit: actually this at() call is just after resize() which can also throw (on all platforms that tesseract supports I assume?) so the fact that there might be an exception should be a non-issue and it protects from UB.

@danpla
Copy link
Contributor

danpla commented Jun 24, 2022

Probably std::vector::data() will help in this particular case.

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.

3 participants