23.03.21
Hab gestern sehr vieles gemacht und geschafft. So viel, dass von den mittleren Punkten für die game-src nur noch etwa 30 Stück übrig sind. Wovon wahrscheinlich auch nur 15 von mir gefixxt werden müssen, der Rest sieht für mich aus wie false positives oder 'Mi Mi Mi's. Aber fangen wir mit den einfachen Dingen auf der Liste an (Es waren 5 Commits oder so, ich gehe einfach ein paar random Änderungen durch):
1. Fehlendes return sorgt für Nutzung von Nullpointern
- if (!g1 || !g2)
- {
- luaL_error(L, "no such guild with id %d %d", gid1, gid2);
- // Hier fehlt das: return 0;
- }
Da hier g1 und g2 auf not null geprüft werden, würde man erwarten das wenn eines der Beiden Pointer null ist, dass dann die Funktion endet. Denn im folgenden passiert:
- PIXEL_POSITION pos;
- uint32_t dwMapIndex = g1->GetGuildWarMapIndex(gid2);
g1 könnt null sein und damit einen Corecrash hervorrufen. Dies wurde in einer vorherigen Tagebuchseite bereits erläutert. Also:
- if (!g1 || !g2)
- {
- luaL_error(L, "no such guild with id %d %d", gid1, gid2);
- return 0;
- }
return hinzugefügt und möglichen Corecrash abgwendet. Natürlich nur unter der Bedingung: Der Server nutzt diese Funktion und man kann sie als Spieler ausführen ohne in einer Gilde zu sein.
2. const references everywhere (Vor allem für große Klassen und Vektoren... Also.. bitte?)
- bool ITEM_MANAGER::SetMaxItemID(const TItemIDRangeTable& range)
- int32_t CDungeon::GetFlag(const std::string& name)
- void CSwitchbot::SetAttributes(uint8_t slot, const std::vector<TSwitchbotAttributeAlternativeTable>& vec_alternatives)
Mehr muss ich glaub nicht mehr sagen, davon gabs wieder etwa 20 Stück, wo das Kopieren (copy constructor) von Objekten in den Stackframe (oder implizites allokieren im Heap z.B bei Strings) problematisch werden könnte bzw. an der Performance nagt.
3. Entfernen von dead code
Schonmal was von einem Yangspeicher bei Gilden, einem Partyskill namens "HEAL_ALL_MEMBERS" oder Lotto in Metin2 gehört? Wenn nicht, ist auch dead code. Wird nie benutzt, am Anfang der Funktion steht dann:
Oder if-Anweisungen wie:
- if (0) // oder if (false) oder if (!!0) oder wie fancy man das auch schreiben mag
Achso und das war kein Spaß, es gibt einen Heilskill bei Metin2 der einfach nie benutzt wird für Gruppen, hier ein Vorschaubild (rechts wäre der Heilskill), funktioniert aktuell nicht bzw. ist 'auskommentiert'.
Bitte melden Sie sich an, um diesen Anhang zu sehen.
4. iter++ ist böse
Was ist das Problem mit folgendem Iterator?
- (; iter != m_member.end(); iter++)
Es werden haufenweise Werte im Stack abgelegt. Glaubt ihr nicht? Dann schauen wir uns mal per Pseudocode den Unterschied zwischen i++ und ++i an.
Was macht ++i? Ganz rudimentär: Ich bekomme einen Wert, so sei i jetzt 3 und inkrementiere ihn um 1 und gebe den neuen Wert nach der Inkrementation also 4 zurück.
(i sei 3)
++i = 4
Jetzt sagt ihr: Moment mal, i++ ist doch auch 4 am Ende der Zeile, was soll das denn jetzt? Dann nochmal das Beispiel mit i++. i++ inkrementiert den Wert, gibt aber den alten Wert, hier also die 3 zurück. Erst am Ende der Zeile bzw. zum Anfang der nächsten Zeile wäre i dann 4.
i++ = 3
// Erst hier ist i = 4
Technisch gesehen könnten die Funktionen also so aussehen:
- // ++i
- // Inkrementiere i und gebe den neuen Wert zurück
- func(&i)
- {
- return i + 1;
- }
- // i++
- // Inkrementiere i, speichere aber den alten Wert und gebe das alte i zurück
- func(&i)
- {
- int32_t tmp = i;
- i += 1;
- return tmp;
- }
Alles anzeigen
Diese Variable tmp wird bei jedem 'i++' erstellt bzw. muss es auch. Ja, alle Compiler die ich kenne werden das on the fly optimieren, aber darauf kann man sich nun Mal nicht immer verlassen. Besser ist, direkt als Entwickler auf die richtigen Operatoren zu achten. Hier noch ein kleines Ratespiel was ich niemals auflösen werde:
- int i = 1;
- int new = i++ + i-- + ++i - ++i + i++ - i + i++;
wie ist der Wert der Variable new am Ende der Zeile?
5. Vermeiden, strings zu kopieren
Eig. ganzt einfach: Ihr erstellt einen String, sagen wir "oldname" und gibt ihm einen Wert. Danach wollt ihr dass der String "newname" den Wert von "oldname" bekommt. Welchen Operator nutzt man instinktiv? "=" - Das mindert die Performance aber. Ihr habt jetzt, wissentlich oder unwissentlich den String von newname komplett neu erstellt, wobei ihr eig. nur den Inhalt von oldname nutzen wolltet. Beispiel gefällig?
- std::string oldname = "Albert";
- std::string newname;
Jetzt gibt es verschiedene Lösungen wie man eine Kopie verhindert. Hier einige davon, die ich nicht näher erläutern möchte weil kb mehr zu schreiben und so:
- newname = std::move(oldname);
- std::swap(newname, oldname);
Jeder dieser Funktionen liefert am Ende das gewünschte Ergebnis, was schneller ist dürft ihr mit dem CPP Bench Tool von der Seite hiervor gern selbst prüfen. Das was sie gemeinsam haben ist:
Nach dieser Zeile, ist oldname eine womöglich leere oder ungülltige Variable. Bei std::move z.B darf man diese Variable danach (ohne sie erneut zu initialisieren) auf keinen Fall benutzen. Der Speicher dieses Objektes wurde 'gemoved'. Man greift damit auf ungülltigen Speicher zu. Bei Assign und Swap ist es ähnlich. Das heißt: Dieser Punkt ist nur relevant, wenn der alte String 'oldname' in diesem Beispiel im Folgenden nicht mehr gebraucht wird.
Puh ich hab doch noch so viel zu erklären, ich hoffe ich hab mich bis jetzt nicht widerholt. Das soll es für heute aber sein, viel Spaß weiterhin mit diesem Thread und statischer Codeanalyse!
