Speicherfehler im 1. elem vom vector



  • @MFK Auch wenn ich Copyconstructor und Destuctor nicht benötige, schaden richten sie keinen an. das points_.clear() stammt von etlichen bisherigen korrektur Versuchen. Mit einem kompilierbaren Code wird schwierig, da noch ein ganzer Rattenschwanz mit dran hängt (Boost-Library, TinyXML oder libpng beispielsweise).

    @ihoernchen habe die Werte zwischengespeichert mit dem gleichen Ergebnis. Hatte ich aber auch bereits vermutet. Denn ich hatte schon eine Variante, in der ich mir erst mTrace result anlege und danach mittels Set-Methoden den Inhalt verändere.



  • @Flaut
    Wenn du den CopyCtor und den Dtor schon mitnimmst, dann halt dich bitte auch an das "Law of the Big Three". Dann ist deine Aussage auch gerechtfertigt 😉 Aber wenn du nur halb halb implementierst, lass es lieber weg oder machs ordentlich.



  • Flaut schrieb:

    @MFK Auch wenn ich Copyconstructor und Destuctor nicht benötige, schaden richten sie keinen an.

    Unnötige Funktionen lenken vom Probem ab, machen den Code unübersichtlicher und bieten zusätzliche Fehlerquellen.

    Verpass deinen Klassen doch auch noch einen Zuweisungsoperator. Wenn schon überflüssiger Code, dann richtig.

    Flaut schrieb:

    Mit einem kompilierbaren Code wird schwierig, da noch ein ganzer Rattenschwanz mit dran hängt (Boost-Library, TinyXML oder libpng beispielsweise).

    Boost und libpng braucht man hier offensichtlich nicht. Und diesen TiXmlElement* durch ein Dummy zu ersetzen, sollte doch auch kein Problem sein.

    Falls doch, dann zeig halt mal mTrace::points().



  • Ich rate dir die ganzen const int Referenzen zu entfernen. Die dienen eher als Fehlerquelle. Bringen im Gegenzug nichts. Unabhängig von deinem Problem.

    Ansonsten wie MFK schon sagte wird es schwer das ganze ohne kompilierbaren Code zu prüfen.

    Fehlerquellen:
    - const int Referenzen
    - nodeToPoint: Was macht nodeToPoint? Bzw. was wird zurückgeliefert?
    - Was passiert, wenn du in nodeToTrace vec mit Dummy-Werten füllst? Geht das immer noch schief?
    - Was passiert im operator<< für mTrace?
    - Wieviele Werte sind im vector (der später ja auch kopiert wird)? Passiert es bei großen wie bei kleinen Mengen? Was passiert, wenn vec genau 1 Element enthält?



  • Sollte die Methode nicht eine konstante Referenz bekommen oder ist die Kopiererei hier gewollt?

    void setPoints( const std::vector<mPoint> vec )
    {
      points_.clear(); 
      points_ = vec;
    }
    

    Und was gibt nodeToPoint() im Fehlerfall zurück? Wenn das erste Element Tünneff ist, kann das deinen Fehler auslösen?

    Gruß Kimmi



  • @MFK: Was meinst Du mit mTrace::points() ?

    ihoernchen schrieb:

    Ich rate dir die ganzen const int Referenzen zu entfernen. Die dienen eher als Fehlerquelle. Bringen im Gegenzug nichts. Unabhängig von deinem Problem.

    gemacht

    ihoernchen schrieb:

    Ansonsten wie MFK schon sagte wird es schwer das ganze ohne kompilierbaren Code zu prüfen.

    ich versuche es zu packen und nen link einzubinden

    ihoernchen schrieb:

    - nodeToPoint: Was macht nodeToPoint? Bzw. was wird zurückgeliefert?

    liest die Datei aus und gibt einen Wert vom Typ mPoint zurück.

    Bsp.
    <Point num="0">4949 1751</Point>
    <Point num="1">4977 1676</Point>
    <Point num="2">5005 1608</Point>

    ihoernchen schrieb:

    - Was passiert, wenn du in nodeToTrace vec mit Dummy-Werten füllst? Geht das immer noch schief?

    ja, gleiche fehler

    ihoernchen schrieb:

    - Was passiert im operator<< für mTrace?

    inline std::ostream& operator<<(std::ostream& out, const mTrace& trace){
    	out << "TraceID: " << trace.traceID() << " Color: " << trace.color()
    		<< " NumPoints: " << trace.numPoints() << "\n";
    	for(std::vector<mPoint>::iterator it = trace.points().begin(); it!=trace.points().end(); ++it){
    		out << *it << '\n';
    	}
    	return out;
    }
    

    ihoernchen schrieb:

    - Wieviele Werte sind im vector (der später ja auch kopiert wird)? Passiert es bei großen wie bei kleinen Mengen? Was passiert, wenn vec genau 1 Element enthält?

    Das variiert, aber im Schnitt 40 Einträge.



  • Flaut schrieb:

    @MFK: Was meinst Du mit mTrace::points() ?

    Deine Klasse mTrace hat eine Methode namens points. Jedenfalls wird sie im Copykonstruktor benutzt.

    Flaut schrieb:

    ihoernchen schrieb:

    - nodeToPoint: Was macht nodeToPoint? Bzw. was wird zurückgeliefert?

    liest die Datei aus und gibt einen Wert vom Typ mPoint zurück.

    Das ist, was die Methode tun soll. Um zu beurteilen, was sie tatsächlich tut, musst du schon den Code zeigen.



  • @kimmi wenn ich vec ausgebe, stimmt der eintrag, aber in result.points_ nicht. Gebe ja beides am Ende der Funktion aus.



  • den Copyconstructor habe ich schon entfernt, um möglichst wenig Fehlercode zu haben. Aber der Vollständigkeit halber.

    /// returns the Points from the Trace
    std::vector<mPoint> points() const {return points_;};
    

    und nodeToPoint

    mPoint nodeToPoint(TiXmlElement* pt) {
    
    	if (pt && strcmp(pt->Value(), "Point") == 0) {
    		const char* c = textValue(pt);
    		int x = 0, y = 0;
    		for (unsigned int i = 0; i < strlen(c); ++i) {
    			if (c[i] == ' ') {
    				x = y;
    				y = 0;
    				++i;
    			}
    			y = 10 * y + (c[i] - 48);
    		}
    		return mPoint(x, y);
    	} else
    		throw runtime_error(string("wrong point ") + pt->Value());
    }
    

    bevor die Frage kommt textValue

    const char* textValue(TiXmlElement* e) {
    
    	TiXmlNode* first = e->FirstChild();
    	if (first != 0 && first == e->LastChild() && first->Type()
    			== TiXmlNode::TEXT)
    		return first->Value();
    	else {
    		throw runtime_error(string("wrong element ") + e->Value());
    	}
    }
    


  • mTrace.points() gibt eine Kopie des Vectors zurück.

    Die Iteratoren, die begin() und end() in deinem operator<< liefern, gehören daher zu unterschiedlichen Kopien des Vectors. Außerdem sind die Iteratoren sofort wieder ungültig, weil die Kopien gleich nach dem Aufruf von points() wieder zerstört werden.

    Lass points() eine konstante Referenz zurückgeben.



  • mh was mir noch auffällt

    // operator<<
    for(std::vector<mPoint>::iterator it = trace.points().begin(); it!=trace.points().end(); ++it)
    
    std::vector<mPoint> points() const {return points_;};
    

    Du gibst eine Kopie des Vectors zurück. Das passt nicht.
    Hier solltest du eine Referenz zurückgeben.

    const std::vector<mPoint>& points() const {return points_;};
    

    Macht Sinn damit der Vector nicht immer kopiert wird.

    Das ist auch das Problem an der Schleife oben. Warum das überhaupt funktioniert ist komisch 🙂

    Edit: zu langsam 🤡



  • Daran hat es tatsächlich gelegen. Funktioniert aber auch nur, da es sich um eine Klasse handelt. Denn bei Funktionen sollten keine Referenzen zurück gegeben werden, da man sonst auf einen verwaisten Speicher zugreift, oder liege ich da auch falsch?

    Super danke.



  • Du darfst keine Referenzen oder Zeiger auf lokale Variablen, die sich innerhalb einer Funktion befinden, zurückgeben. Grund: nach verlassen der Funktion ist die Variable weg.
    Dabei ist es egal ob Methode (einer Klasse) oder ('freie') Funktion.

    // böse
    MyClass& Funktion()
    {
      MyClass m;
      return m;
    }
    MyClass* Funktion()
    {
      MyClass m;
      return &m;
    }
    
    // ok
    MyClass& Funktion()
    {
      static MyClass m; // statische Variablen bleiben bis zum Programmende
      return m;
    }
    MyClass* Funktion()
    {
      static MyClass m; // ginge natürlich auch: MyClass* m = new MyClass; return m;
      return &m;
    }
    
    // auch ok
    MyClass m;
    MyClass& Funktion() // selbes für MyClass*
    {
      return m;
    }
    


  • ihoernchen schrieb:

    // ginge natürlich auch: MyClass* m = new MyClass;
    

    Davon würde ich sehr stark abraten, besonders wenn es anders geht. Die Wahrscheinlichkeit, dass der Aufrufer vergisst, den Speicher freizugeben, ist einfach zu hoch. Mindestens ein Smart Pointer oder besser gleich eine Objektrückgabe wäre eine Alternative.


Anmelden zum Antworten