memory errors in 376c8c65fcc33eb31541a971e6c8b7e9c2013f18

[wzdev-ci]

resolution_fixed type_bug | by Reg312

---

I'm using VS2010 for compiling warzone for my debug experiments Source file stats.cpp was changed in latest commints in master-version (converting stats to ini) since this changes i cannot run warzone, getting memory errors!

Also i found way to fix this memory erros i just need add strdup() calls to places where QT string get converted to c-string e.g. weaponName = ini.value("turret").toString().toUtf8().constData(); weaponName = strdup(ini.value("turret").toString().toUtf8().constData());

after this changes i can run warzone.

---

Issue migrated from trac:3821 at 2022-04-16 10:44:31 -0700

[wzdev-ci]

vexed changed priority from normal to blocker

[wzdev-ci]

vexed changed blocking which not transferred by tractive

[wzdev-ci]

vexed changed blockedby which not transferred by tractive

[wzdev-ci]

vexed changed title from memory errors in Warzone compiled in VS2010 (related to new ini-stats and QT) to memory errors in 376c8c65fcc33eb31541a971e6c8b7e9c2013f18

[wzdev-ci]

vexed commented

---

This has nothing to do with VS2010. This is a case of not understanding what is being done behind the scenes, and what you should and shouldn't do when using Qsettings.

I'll do a quick fix shortly.

[wzdev-ci]

vexed uploaded file 0003-Don-t-assign-memory-that-has-been-previously-freed-y.patch (9.1 KiB)

[wzdev-ci]

vexed changed _comment0 which not transferred by tractive

[wzdev-ci]

vexed commented

---

Should work for all platforms. Test please.

Reg312, you would leak memory by doing that.

You can't just assign a byte array to a const char since the destructor is executed before the assignment. So you end up assigning memory that was freed.

refs #3794 refs #3795 (I have no idea which is what and where this patch came from)

Broken in: Revision: 376c8c65fcc33eb31541a971e6c8b7e9c2013f18 Author: felippico

[wzdev-ci]

Reg312 commented

---

vexed, i know about memory leak in my solution, i just dont understand some things to find better way e.g. i dont see where is destructor called for byteArray in your patch? looks like QT should care about this.

Tested patch: it works. stats was loaded without errors.

[wzdev-ci]

NoQ commented

---

Emm, but it's a stack variable. The destructor is called automatically when the function returns, right?

[wzdev-ci]

Reg312 commented

---

2NoQ: may be i created QByteArray-variable and put it to some list, in that case i dont want destructor called. as far i understand stack variable is just pointer to memory allocated by QByteArray class. so i see following possibilites case 1: QT uses garbage collector case 2: QT automatically call destructor for such variables on function return. But how it determines more permanent stuff? case 3: we need call destructor for objects we have created ...i hope someone can clarify :)

[wzdev-ci]

Per commented

---

string.toUtf8().constData() gives you a temporary c-string reference that must be used immediately.

QByteArray is allocated on the stack, and when it leaves scope it will call its own destructor, that's why there is no memory leaks with vexed's patch. However, I'd rather we used QString than QByteArray here.

I should have caught this error during review. Sorry. Will commit fix.

[wzdev-ci]

Per uploaded file fixmemorybug.diff (24.3 KiB)

Another fix based on QString, also fixes some more memory leaks.

[wzdev-ci]

vexed commented

---

It compiles and it don't crash, and looks saner.

[wzdev-ci]

Reg312 commented

---

tested fixmemorybug.diff​ - ok, compiles and properly load stats

*just noticed oil derrick have 100$ price in master

[wzdev-ci]

Per changed status from new to closed

[wzdev-ci]

Per changed resolution from ` tofixed`

[wzdev-ci]

Per commented

---

Fixed in 97fe0b62b9ab206275c403be19bc483cbc8cdb98