C++ Gültigkeit von Variablen/Objekten

[BlackMark]

Wenn ja, könnte da das Problem liegen: Im Code hat der Copy Operator move semantics implementiert. Weird und gefährlich imho. Das würde afaik nämlich DataXYZ ungültig machen ohne std::move zu nutzen.

Ganz genau. Das ist absolut nicht wie man so etwas implementieren darf. Dieser DataXYZ Typ und alles andere das auf DataPointer aufbaut ist absolut schrecklich. Man muss es moven, weil wenn eine Kopie out-of-scope geht, werden alle anderen Kopien ungültig (aber auch nur, wenn der destructor von m_private_impl auch den m_pointer freed, ansonsten leaked hier alles).

  inline void  shallowCopy(const DataPointer& other) {
    if(m_private_impl != 0)
      delete m_private_impl; //// Wieso? wird dadurch nicht der Speicher gelöscht, sodass er auch in der Kopie nicht mehr verfügbar ist?
    m_pointer = other.m_pointer;
    m_size = other.m_size;

    m_private_impl = other.m_private_impl;
  }

Das passt schon so. delete m_private_impl stellt sicher, dass der alte Wert vom Empfänger der Kopie gelöscht wird. Es wird ja nicht delete other.m_private_impl aufgerufen, das wäre ein Problem.

Gruß
BlackMark

 

[new Account()]

Das passt schon so. delete m_private_impl stellt sicher, dass der alte Wert vom Empfänger der Kopie gelöscht wird. Es wird ja nicht delete other.m_private_impl aufgerufen, das wäre ein Problem.

Ah stimmt, nicht richtig gelesen.

 

[BlackMark]

Habe mir gerade das ganze Repo geladen und mal durch die Implementierung geschaut. Scheinbar ist es kein Problem wenn eine Kopie von DataXYZ out-of-scope geht, weil die Ownership vom m_pointer bei Scan liegt und das ein static std::vector<Scan*> allScans; hat. Also solange die Scans nicht gelöscht werden, sind auch alle DataXYZ gültig. Großartig. So funktioniert Ownership.

@te one Dein Problem liegt dann wahrscheinlich darin, dass deine Daten schon vorher ungültig sind.

Gruß
BlackMark

 

[te one]

@BlackMark
Beim Schreiben passen die Werte aber anfangs und werden später erst invalide.
Jetzt pass auf, ich habe einen Ansatz
Ich mache das ganze mit einer Loop wie folgt:

vector<int> neededReductions; // Usually this vector is filled with data by the user (command line). We use 10 and 20 as an example.
neededReductions.push_back(10);
neededReductions.push_back(20);
// Usually for each scan... Here I only show it for one scan
for (unsigned int i=0;i<neededRecutions.size();i++) {
    scan->setReductionParameter(neededReductions[i], 1, pointtype);
    scan->calcReducedPoints();
    DataXYZ xyz(scan->get("xyz reduced"));
    reducedPointsMap.insert({neededReductions[i],xyz});
}

Das heißt ich packe jedesmal wieder meine neuen Reduction-Parameter in das Scan-Objekt und hole mir ein neues (?) DataXYZ-Objekt mit den entsprechend reduzierten Daten.
Kann es sein, dass ich mir hier ab dem zweiten Schleifendurchlauf die Daten zerstöre?

 

[BeBur]

Ja, komme wohl wirklich nicht drumherum mal ein Beispiel ohne externen Code zu bauen.

Kann ich sehr empfehlen. Ich habe fast noch nie eine Frage gestellt, weil spätestens wenn ich ein Minimalbeispiel konstruiert habe habe ich in aller Regel heraus gefunden, was das Problem ist.

 

[te one]

Kann ich sehr empfehlen. Ich habe fast noch nie eine Frage gestellt, weil spätestens wenn ich ein Minimalbeispiel konstruiert habe habe ich in aller Regel heraus gefunden, was das Problem ist.

Ja, verstehe das schon. Habe mittlerweile auch ein kleines Beispiel gebaut (mit eigenen kleinen Klassen statt den Scans aus dem Git-Projekt). In dem Test läuft alles wie gedacht. Das nun realitätsnaher zu bauen ist nur jetzt schwierig. Das Problem titt nur bei manchen Eingabedaten auf (in dem Fall ein Scan mit einigen Millionen Punkten und verschiedenen Reductions). Das kann man nicht mal eben auf Korrektheit validieren...
Aber ich werde mich da nochmal dran setzen.

Was ist mit meiner Vermutung, dass der mehrfache Aufruf von

scan->setReductionParameter(neededReductions[i], 1, pointtype);
scan->calcReducedPoints();
DataXYZ xyz(scan->get("xyz reduced"));

die früheren DataXYZ ungültig macht. Ein Ansatz oder völlig daneben?

Denke du einem funktionstüchtigen Beispielcode komme ich nicht vor nächster Woche.

 

[BlackMark]

Kann es sein, dass ich mir hier ab dem zweiten Schleifendurchlauf die Daten zerstöre?

Ich würde sagen das könnte durchaus so sein. Der Code von dieser komischen Library ist nicht gerade schön geschrieben und es gibt zwei Implementierungen von Scan und ich weiß nicht welche du verwendest, aber ich halte es für nicht ausgeschlossen, dass ein weiterer Aufruf von scan deine alten Ergebnisse löscht/überschreibt/invalidiert.

Ich würde vorschlagen du baust dir ein Objekt welches eine wirkliche Kopie von einem DataXYZ Objekt erstellt und arbeitest mit dem weiter.

So etwas:

class CopiedDataXYZ
{
public:
    CopiedDataXYZ(const DataXYZ& xyz)
    {
        // TODO: Implement
    }
    
private:
    std::vector<double> m_data;
};

Gruß
BlackMark

 

[simpsonsfan]

Wäre sowas nicht ein Fall, wo man mal mit Valgrind die Speichernutzung überprüfen sollte?

 

[te one]

So, habe mir das ganze heute nochmal angesehen. Tatsächlich ändern sich diese DataXYZ und DataType (erben von DataPointer), sofern ich mir später anders reduzierte Daten erzeuge. Ein schönes Kopieren dieser Objekte habe ich auch nicht hinbekommen.
Daher mein Workaround:
Ich schreibe alles direkt in vector<T> weg (in einer for-loop). Da weiß ich, wie sich die Klassen verhalten Und die enthaltenen int/doubles/... in den Vektoren bleiben natürlich auch über den kompletten Programmlauf unverändert.

Habe noch nicht alles umgebaut, aber es sieht recht vielversprechend aus. Hätte gerne verstanden wo das Problem liegt, aber wenn selbst ihr das recht undurchsichtig findet, gebe ich mich als C++-Anfänger auch mit dem Workaround zufrieden

Danke für die zahlreiche Hilfe. Ich denke/hoffe jetzt läuft bald alles

 

[BlackMark]

Ein schönes Kopieren dieser Objekte habe ich auch nicht hinbekommen.

Wieso? Wenn du weißt wie du die Daten aus einem DataXYZ raus bekommst kannst du dir ja ganz einfach eine CopiedDataXYZ Klasse schreiben, die das für dich macht. Du musst nur die Vorlage die ich in Post #27 geschrieben habe ausimplementieren. Dann kannst du deinen ursprünglichen Code genau so verwenden wie er war, nur dass du CopiedDataXYZ in deinen Vektoren und Maps haltest.

Hätte gerne verstanden wo das Problem liegt, aber wenn selbst ihr das recht undurchsichtig findet, gebe ich mich als C++-Anfänger auch mit dem Workaround zufrieden

Das Problem ist eigentlich ganz einfach zu verstehen. Die Library die du verwendest hält sich nicht an etablierte C++ Guidelines und bricht das principle of least astonishment. Du bekommst über einen Scan Daten durch ein DataXYZ Objekt und kopierst es, was ein vollkommen legaler und sinnvoller Vorgang ist. Die Library implementiert jedoch den Copy-Constructor falsch und macht eine shallow-copy (non-destructive move), also nur eine weitere Referenz auf die ursprünglichen Daten, keine Kopie.
Außerdem wird Ownership falsch modelliert, eigentlich sollte ein DataXYZ Objekt Ownership über die Daten die es beinhaltet haben, das ist jedoch nicht so, Ownership hat das Scan Objekt durch welches die Daten erzeugt wurden. Wenn du also das Scan Objekt veränderst, verändern sich auch die Daten in DataXYZ, entweder weil sie gelöscht werden, oder weil neue Daten rein geschrieben werden (genaue Details sind schwer aus dem Library Code auszulesen).
Das Problem liegt also eigentlich an der Library und nicht an dir, dein Code war prinzipiell richtig und würde mit einer normalen C++ Library auch funktionieren.

War das verständlich? Ownership ist ein Begriff für dich? Falls nicht solltest du das nachlesen, das ist unglaublich wichtig in C++. Genauso wie copy und move semantics.

Gruß
BlackMark