Skip to content

[GEN][ZH] Fix undefined behavior in ThingTemplate::isEquivalentTo#932

Merged
xezon merged 8 commits into
TheSuperHackers:mainfrom
Caball009:fix_this_pointer_thing_template
Jul 12, 2025
Merged

[GEN][ZH] Fix undefined behavior in ThingTemplate::isEquivalentTo#932
xezon merged 8 commits into
TheSuperHackers:mainfrom
Caball009:fix_this_pointer_thing_template

Conversation

@Caball009

Copy link
Copy Markdown

Another case of checking 'this' pointer in a member function like #870 & #878

Bool ThingTemplate::isEquivalentTo(const ThingTemplate* tt) const
{
// sanity
if (!(this && tt))
return false;

This pr fixes the undefined behavior in ThingTemplate::isEquivalentTo by making it a static function.

If the changes are considered acceptable, then it needs to be replicated in Generals. As far as I'm concerned that can be done in this pr as well.

Comment thread GeneralsMD/Code/GameEngine/Source/GameLogic/AI/AIPlayer.cpp Outdated
Comment thread GeneralsMD/Code/GameEngine/Source/GameLogic/AI/AISkirmishPlayer.cpp Outdated
Comment thread GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Update/ProductionUpdate.cpp Outdated
@Mauller

Mauller commented May 25, 2025

Copy link
Copy Markdown

The better solution here might be to get rid of the equivalent to function and Instead implement an overload for the comparison operator within thingtemplate.

Then do (object->getTemplate() == otherTemplate)

Comment thread GeneralsMD/Code/GameEngine/Source/Common/Thing/ThingTemplate.cpp Outdated
@xezon xezon added Minor Severity: Minor < Major < Critical < Blocker Refactor Edits the code with insignificant behavior changes, is never user facing labels May 25, 2025
@Caball009

Copy link
Copy Markdown
Author

The better solution here might be to get rid of the equivalent to function and Instead implement an overload for the comparison operator within thingtemplate.

Then do (object->getTemplate() == otherTemplate)

I like that approach.

@xezon

xezon commented May 25, 2025

Copy link
Copy Markdown

The change also needs to be replicated in Generals

@Caball009 Caball009 force-pushed the fix_this_pointer_thing_template branch 2 times, most recently from 9dad48b to d914978 Compare July 10, 2025 14:48
@Caball009 Caball009 force-pushed the fix_this_pointer_thing_template branch from d914978 to 5c33dbb Compare July 10, 2025 14:51
@Caball009

Caball009 commented Jul 10, 2025

Copy link
Copy Markdown
Author

Updated and replicated in Generals.

I made the assumption that the ThingTemplate cannot be NULL for Objects and PartitionFilterThings. Afaik, objects are only created in ThingFactory::newObject, which checks if the template is NULL. I checked and all calls to PartitionFilterThings constructor are behind a template != NULL check. I added an assertion to the constructor for good measure.

@Caball009 Caball009 requested a review from xezon July 10, 2025 15:07

@xezon xezon left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Looks very safe.

@xezon xezon changed the title [ZH] Fix undefined behavior in ThingTemplate::isEquivalentTo [GEN][ZH] Fix undefined behavior in ThingTemplate::isEquivalentTo Jul 12, 2025
@xezon xezon merged commit 808017e into TheSuperHackers:main Jul 12, 2025
15 checks passed
@Caball009 Caball009 deleted the fix_this_pointer_thing_template branch July 12, 2025 16:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Minor Severity: Minor < Major < Critical < Blocker Refactor Edits the code with insignificant behavior changes, is never user facing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants