1dot13/source
1
0
Fork 0
mirror of https://github.com/1dot13/source.git synced 2026-08-26 14:30:26 +02:00
Code Issues Packages Projects Releases Wiki Activity

take popup option names by const reference, not by pointer

Browse Source 
POPUP_OPTION copies the name it is given -- `this->name = *newName` into a
std::wstring member -- but its constructor took a std::wstring*, so 64 call
sites spelled the argument `&std::wstring( pStr )`. Taking the address of a
temporary is ill-formed, and clang refuses it outright:

    error: taking the address of a temporary object of type 'std::wstring'

It happens to work under MSVC because the temporary outlives the call, dying
at the end of the full expression rather than before the copy. Nothing was
corrupt; the code was just spelling "pass me a string" in a way the language
does not allow.

A const reference says what these functions actually want, so the call sites
lose the &, and the two places that allocated a string purely to have an
address to pass -- POPUP_SUB_POPUP_OPTION's default constructor, which carried
a "TODO: possible memmory leak!" saying as much, and a "Dummy generator"
option -- stop leaking one.

Converted: POPUP_OPTION's constructor and setName, POPUP::addOption and
addSubMenuOption, and both POPUP_SUB_POPUP_OPTION constructors. The popupDef
family in popup_definition.* keeps its std::wstring* because it owns what it
is handed and stores the pointer; that is a different design and a different
change. The three calls it makes into the converted API now dereference.

Verification:
    grep -rn '&std::wstring' --include=*.cpp --include=*.h .   # nothing
    ninja -C build parse             # no popup_* or SkillMenu sites remain
    ninja -C build -k 0              # Release, four applications, green
    ninja -C build-debug -k 0        # Debug, four applications, green

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
Marco Antonio J. Costa
2026-07-23 19:29:55 -03:00
committed by majcosta
co-authored by Claude Opus 4.8
parent 83b637aa73
commit 5e2982c509
7 changed files with 96 additions and 96 deletions
Download Patch File Download Diff File Expand all files Collapse all files
Utils/popup_class.cpp
+9 -9
View File
@@ -140,9 +140,9 @@ POPUP_OPTION::~POPUP_OPTION(void)
}
POPUP_OPTION::POPUP_OPTION(std::wstring *newName, popupCallback * newFunction)
POPUP_OPTION::POPUP_OPTION(const std::wstring& newName, popupCallback * newFunction)
{
this->name = *newName;
this->name = newName;
this->action = newFunction;
this->avail = 0;
@@ -161,9 +161,9 @@ POPUP_OPTION::POPUP_OPTION(std::wstring *newName, popupCallback * newFunction)
this->color_shade = FONT_GRAY7 ;
}
BOOLEAN POPUP_OPTION::setName( std::wstring * newName )
BOOLEAN POPUP_OPTION::setName( const std::wstring& newName )
{
this->name = *newName;
this->name = newName;
return TRUE;
}
@@ -236,19 +236,19 @@ BOOLEAN POPUP_OPTION::forceRun()
//////////////////////////////////////////////////////////////////
// constructor
POPUP_SUB_POPUP_OPTION::POPUP_SUB_POPUP_OPTION(void) : POPUP_OPTION(new std::wstring(L"Unnamed subPopup"),NULL) //TODO: possible memmory leak!
POPUP_SUB_POPUP_OPTION::POPUP_SUB_POPUP_OPTION(void) : POPUP_OPTION(L"Unnamed subPopup", NULL)
{
this->parent = NULL;
this->initSubPopup();
}
POPUP_SUB_POPUP_OPTION::POPUP_SUB_POPUP_OPTION(std::wstring* name) : POPUP_OPTION(name, NULL)
POPUP_SUB_POPUP_OPTION::POPUP_SUB_POPUP_OPTION(const std::wstring& name) : POPUP_OPTION(name, NULL)
{
this->parent = NULL;
this->initSubPopup();
}
POPUP_SUB_POPUP_OPTION::POPUP_SUB_POPUP_OPTION(std::wstring* newName, const POPUP * parent) : POPUP_OPTION(newName, NULL)
POPUP_SUB_POPUP_OPTION::POPUP_SUB_POPUP_OPTION(const std::wstring& newName, const POPUP * parent) : POPUP_OPTION(newName, NULL)
{
this->parent = parent;
this->initSubPopup();
@@ -562,7 +562,7 @@ void POPUP::setInitialValues(void)
// setup functions
POPUP_OPTION * POPUP::addOption(std::wstring * name, popupCallback* action)
POPUP_OPTION * POPUP::addOption(const std::wstring& name, popupCallback* action)
{
if (this->optionCount < POPUP_MAX_OPTIONS)
{
@@ -605,7 +605,7 @@ POPUP_OPTION * POPUP::getOption(UINT16 n)
return NULL;
}
POPUP * POPUP::addSubMenuOption(std::wstring * name)
POPUP * POPUP::addSubMenuOption(const std::wstring& name)
{
if (this->subPopupOptionCount < POPUP_MAX_SUB_POPUPS)
{
Utils/popup_class.h
+6 -6
View File
@@ -67,10 +67,10 @@
{
public:
POPUP_OPTION::POPUP_OPTION(void); // default constructor
POPUP_OPTION(std::wstring* name, popupCallback* newFunction); // constructor
POPUP_OPTION(const std::wstring& name, popupCallback* newFunction); // constructor
~POPUP_OPTION(); // destructor
// setup
BOOLEAN setName(std::wstring * name);
BOOLEAN setName(const std::wstring& name);
BOOLEAN setAction(popupCallback*fun);
BOOLEAN setAvail(popupCallback *fun);
BOOLEAN setHover(popupCallback *fun);
@@ -105,8 +105,8 @@
public:
// constructor/destructor
POPUP_SUB_POPUP_OPTION(void);
POPUP_SUB_POPUP_OPTION(std::wstring* name);
POPUP_SUB_POPUP_OPTION(std::wstring* newName, const POPUP * parent);
POPUP_SUB_POPUP_OPTION(const std::wstring& name);
POPUP_SUB_POPUP_OPTION(const std::wstring& newName, const POPUP * parent);
~POPUP_SUB_POPUP_OPTION();
void showPopup();
@@ -139,7 +139,7 @@
POPUP(CHAR* name); // constructor
~POPUP(void); // destructor
// setup
POPUP_OPTION * addOption(std::wstring * name, popupCallback * action);
POPUP_OPTION * addOption(const std::wstring& name, popupCallback * action);
/*INT16 findFreeOptionIndex();*/
BOOLEAN addOption(POPUP_OPTION &option);
POPUP_OPTION * getOption(UINT16 n);
@@ -147,7 +147,7 @@
BOOLEAN delOption(CHAR* name); // Another index to through and clean, aargh
BOOLEAN delOption(UINT8 optIndex);
POPUP* addSubMenuOption(std::wstring * name);
POPUP* addSubMenuOption(const std::wstring& name);
BOOL addSubMenuOption(POPUP_SUB_POPUP_OPTION* sub);
/*INT16 findFreeSubMenuOptionIndex();*/
POPUP_SUB_POPUP_OPTION * getSubPopupOption(UINT8 n);
Utils/popup_definition.cpp
+3 -3
View File
@@ -108,7 +108,7 @@
POPUP_OPTION * opt = new POPUP_OPTION();
opt->setName( this->name );
opt->setName( *this->name );
if ( !setPopupDefCallback(opt, this->callbackId)
|| !setPopupDefAvail(opt, this->availId) )
{
@@ -132,7 +132,7 @@
*/
BOOL popupDefSubPopupOption::addToBox(POPUP * popup){
POPUP_SUB_POPUP_OPTION * sub = new POPUP_SUB_POPUP_OPTION( this->name );
POPUP_SUB_POPUP_OPTION * sub = new POPUP_SUB_POPUP_OPTION( *this->name );
if( !this->content->applyToBox( sub->subPopup ) ){
delete sub;
@@ -169,7 +169,7 @@
switch(generatorId){
case popupGenerators::dummy:
popup->addOption(new std::wstring( L"Dummy generator" ),NULL);
popup->addOption(L"Dummy generator", NULL);
break;
case popupGenerators::addArmor: