8000 Blockmap Size Error Checking (Fixes bug #1287) by MistUnky · Pull Request #1289 · chocolate-doom/chocolate-doom · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Blockmap Size Error Checking (Fixes bug #1287) < 8000 span class="f1-light color-fg-muted">#1289

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.

Sign up for GitHub

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

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
Open

Blockmap Size Error Checking (Fixes bug #1287) #1289

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
1ec7a37
Add blockmap size error for doom/p_setup.c
MistUnky Jun 3, 2020
3d7b292
Add blockmap size error for heretic/p_setup.c
MistUnky Jun 3, 2020
b327032
Add blockmap size error for hexen/p_setup.c
MistUnky Jun 3, 2020
4703e60
Add blockmap error for strife/p_setup.c 8000
MistUnky Jun 3, 2020
ca54eb5
Fix style
MistUnky Jun 3, 2020
c802824
Fix style
MistUnky Jun 3, 2020
03aec42
Fix style
MistUnky Jun 3, 2020
31aab80
Fix style
MistUnky Jun 3, 2020
ea59b6c
Fix style (Completes fixes for bug #1287)
MistUnky Jun 3, 2020
7ef3f6e
Merge branch 'master' of https://github.com/MistUnky/chocolate-doom
MistUnky Jun 3, 2020
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
4 changes: 4 additions & 0 deletions src/doom/p_setup.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -531,6 +531,10 @@ void P_LoadBlockMap (int lump)

lumplen = W_LumpLength(lump);
count = lumplen / 2;
if(count >= 32766)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why exactly this number?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's the blockmap limit for doom / 2 (count = blockmap size / 2, and blockmap limit is 65536 in doom, so I put that /2 and checked it against count.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Source: https://doomwiki.org/wiki/Blockmap#Compression
and C data type size for short (which is what blockmap is) is 65536

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you, but this was a rhetorical question. 😉 I'd prefer if magic numbers in source code didn't "fall from the sky". Something like USHRT_MAX/2 and an accompanying comment would probably be more meaningful.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, 32766 is the wrong number since 65536 / 2 = 32768. A good example of why Fabian's approach is the better one.

{
I_Error("Blockmap too large");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not a good error message. A better error message would include the two values that were compared and a link to the doomwiki.org page about the static limit.

}

blockmaplump = Z_Malloc(lumplen, PU_LEVEL, NULL);
W_ReadLump(lump, blockmaplump);
Expand Down
4 changes: 4 additions & 0 deletions src/heretic/p_setup.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -429,6 +429,10 @@ void P_LoadBlockMap(int lump)
int lumplen;

lumplen = W_LumpLength(lump);
if((lumplen / 2) >= 32766)
{
I_Error("Blockmap too large");
}

blockmaplump = Z_Malloc(lumplen, PU_LEVEL, NULL);
W_ReadLump(lump, blockmaplump);
Expand Down
4 changes: 4 additions & 0 deletions src/hexen/p_setup.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -539,6 +539,10 @@ void P_LoadBlockMap(int lump)
int lumplen;

lumplen = W_LumpLength(lump);
if((lumplen / 2) >= 32766)
{
I_Error("Blockmap too large");
}

blockmaplump = Z_Malloc(lumplen, PU_LEVEL, NULL);
W_ReadLump(lump, blockmaplump);
Expand Down
4 changes: 4 additions & 0 deletions src/strife/p_setup.c
8000
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -510,6 +510,10 @@ void P_LoadBlockMap (int lump)

lumplen = W_LumpLength(lump);
count = lumplen / 2;
if(count >= 32766)
{
I_Error("Blockmap too large");
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Respect the code style. For example, put a space between if and (. Also, don't use tabs for indentation.

Code style: https://github.com/chocolate-doom/chocolate-doom/blob/master/HACKING.md

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1


blockmaplump = Z_Malloc(lumplen, PU_LEVEL, NULL);
W_ReadLump(lump, blockmaplump);
Expand Down
0