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

Proposal: Replace //PARANOID with assert #72

Open
cochrane opened this issue Jun 11, 2015 · 0 comments
Open

Proposal: Replace //PARANOID with assert #72

cochrane opened this issue Jun 11, 2015 · 0 comments
Milestone

Comments

@cochrane
Copy link
Contributor

There are a number of places in the code where some invariant is enforced with a comment saying something like //PARANOID! See for example:

GLubyte *mip_data = (GLubyte *) malloc(4 * w * h);

w = (w==0)?1:w; ///@PARANOID: tex atlas size can't been less or equal 2 x 2
h = (h==0)?1:h;
for(int i=0;i<h;i++)
{
    for(int j=0;j<w;j++)
    {
        mip_data[i * w * 4 + j * 4 + 0] = 0.25 * ((int)data[i * w * 16 + j * 8 + 0] + (int)data[i * w * 16 + j * 8 + 4 + 0] + (int)data[i * w * 16 + w * 8 + j * 8 + 0] + (int)data[i * w * 16 + w * 8 + j * 8 + 4 + 0]);
…

from bordered_texture_atlas.cpp.

Doing that is wrong. Imagine a scenario where w and h are 0. In that case, we enter the loop, and write to mip_data… but mip_data has a length of zero at that point so we can't write to it. We have a bug here either way. (If you look at the full code you'll also see that our access to data has a bug as well, since the first iteration will access data[4], but it will only have a total size of four at that point. But that's just for bonus points.)

If we get to a situation where this //PARANOID code actually does something, then we're already in trouble. Some part of the code messed up, and now we're dealing with invalid data. This paranoid stuff allows the code to limp along a little bit further, so it can then crash somewhere else, where nobody knows where it's coming from.

The solution? assert. In that case,

assert(w > 0 && h > 0);

Plain and simple. If the condition is violated, we get a crash, exactly at the point where we got the wrong data, and can debug it. The app never enters an undefined state.

cochrane added a commit that referenced this issue Jun 11, 2015
And replace one //PARANOID with assert, as announced in #72.
@stohrendorf stohrendorf added this to the TR1 milestone Sep 2, 2016
TeslaRus pushed a commit that referenced this issue May 1, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants