ZDoom

Why did you revert the const reference part, BTW?

Because FNames are supposed to be passed as values, they are 4-byte so passing them as reference is more inefficient.

In 'FState *FStateDefinitions::ResolveGotoLabel (AActor *actor, const PClass *mytype, char *name)', what the 'actor' parameter is supposed to do? The routine assigns it a value that is neither can be used outside of it (no deference of the pointer) nor is used anywhere inside it.

EDIT: I was talking about TArray<T>, not FName parameters

ADDEDNUM: in 'static visplane_t *new_visplane (unsigned hash)',

I just left that file as it was because I couldn't see the benefit of your changes for these inline function. That was just 'if it ain't broke, don't fix it'.

For ResolveGotoLabel I'd really like to tell you what it's used for - but it looks like some leftover development garbage.

BTW const ref in info.h at line 220 wasn't reverted. HEAD doesn't build because of that, and pull request with the fix was rejected. Did you forget to push this change?

Yup

https://github.com/rheit/zdoom/pull/265 takes care of it.

Ok, so far.

I made two changes:

- the pointer you freed in d_net.cpp needs to be NULLed afterward.
- buildmap.cpp contains code solely for data format reference so I discarded your changes to this file.

ZzZombo wrote:

Code: Select all

		check = (visplane_t *)M_Malloc (sizeof(*check) + 3 + sizeof(*check->top)*(MAXWIDTH*2)); <- NULL pointer dereference I don't know how to fix myself.

That is not a NULL pointer dereference, since they happen in a sizeof operator (the code is not evaluated, all it does is check the resulting type for its size at compile time).

Oh yeah, thanks!

Changes in d_net.cpp causes a crash at line 2054. I think you forgot to set m_BufferLen to zero when the buffer was freed.
To reproduce this crash: load any map and switch between weapons quickly, e.g. by rapidly pressing keys bound to weapon slots 1 and 2.

I guess that might explain this issue as well, maybe? Dunno if it's related enough.

Major Cooke wrote:I guess that might explain this issue as well, maybe? Dunno if it's related enough.

The last screenshot looks familiar. At least memcpy() is there. Cannot say for sure, as you didn't load debugging symbols.

Ugh...

You know what?
After careful examination I decided to restore this part to what it was BEFORE this failed sanitation attempt, i.e. I don't free the buffer and only set the used length to 0.
I also will close any further sanitation attemps that seem to come from reacting to the output of code analysis tools. They cause more work than they are worth.

Was there really a solid reason why this was even done in the first place? Pretty sure ZDoom was working fine up to this point...

I haven't updated my fork's code base in quite a while, I hope there wont' be any surprises waiting for me... :P