Verweis uaf eine gelöschte Funktion in std::unique_ptr

&(*null)->Hallo schrieb:

Dennoch frage ich mich, wann Du das Object gelöscht hast, weswegen die Fehlermeldung kam. Irgendwann hast Du einen Scope verlassen, weswegen der unique_ptr-Deleter aufgerufen worden ist,

Du hast die Fehlermeldung nicht richtig gelesen oder verstanden.

Quaneu schrieb:

All meine Daten sollen "immutable" sein, da ich eine Datei lese und diese nur anzeigen will. Daher will ich gar nicht, dass sie danach manipuliert werden.

Die Klasse Abstract ist eine Basisklasse für viele andere Klassen, daher benutze ich eigentlich auch den std::unique_ptr, wegen polymorphie.

Ja, aber mit folgendem:

std::vector<std::unique_ptr<Abstract>> Get() const { return _items; }

erzeugst du eine KOPIE des Vektors! Du versuchst also beim Aufruf, den vector samt Inhalt komplett zu duplizieren. D.h. du versuchst somit auch, die unique_ptr zu kopieren.

Eigentlich willst du ja nur, dass man den Inhalt sieht und gar keine Kopie erzeugen. Daher einfach folgendes schreiben:

const std::vector<std::unique_ptr<Abstract>>& Get() const { return _items; }

Oder aber du machst es mehr im STL-Stil und machst begin und end des vectors nach außen verfügbar.

Wenn man den Vector nicht als Referenz übergeben würde, würde der Aufruf Get() auch nur einmal funktionieren => Man muss es als Referenz zurück geben.
Danke schon mal dafür.

Update:
Wenn ich nun eine Referenz zurück gebe geht es leider auch nicht, sobald ich eine Instanz von Example anlege.

const std::vector<std::unique_ptr<Abstract>>& Get() const { return _items; };

main.cpp
#include "Example.h"

int main()
{
	Example example;
	auto items = example.Get();
	return 0;
}

Damals hatte ich noch eine Vermutung, aber jetzt weiß ich überhaupt nicht mehr woran dies liegen kann...

unskilled

auto items = example.Get();
denk mal nach, was hier passiert.

Also die Funktion gibt eine Referenz zurück, dabei sollte nichts "schlimmes" passieren, danach wird zugewiesen und das sollte hoffentlich nicht mit kopieren passieren. Aber ich denke das stimmt nicht. Was pssiert denn und wie macht man es denn richtig?

Gleichheitszeichen machen was? Richtig, Kopien!
Wieso sollte = nicht kopieren? Wenn du a=b; schreibst mit zwei ints, erwartest du doch auch eine Kopie, nach der a von B unabhängig ist.

Mit dieser Anweisung versucht du also erneut, den vector zu kopieren.

Wenn du die Referenz, die rechts des =-Zeiches steht, behalten willst, musst du das explizit mitteilen:

auto &items = example.Get();

Dadurch kopierst du nur die Referenz, aber nicht den Inhalt.

Ich kann nur raten, mal einfach ein wenig mit ints herumzuspielen, z.B. so

int main (){
  int a = 42;
  int &aref = a;
  ... Hier irgendwie die Variablen manipulieren ...
  std::cout << a << '\n';
}

Braunstein

Ich habe das Gefühl, dass die Verwendung von auto bei Anfängern eher Verwirrung stiftet. Vielleicht wäre es für den Anfang gut einfach den gewünschten Typen hinzuschreiben.

const std::vector<std::unique_ptr<Abstract>>& items = example.Get();

Quaneu

Ich habe mir eben aus diesem Grund ein kleines Beispiel gebaut und habe da auch so einiges ausprobiert. Aber leider bin ich von falschen Tatsachen ausgegangen, da ich dachte das bei = entweder kopiert oder gemoved wird.

Mittlerweile zweifel ich jedoch grundsätzlich an der Verwendung von unique_ptr in einem Vector, wenn dieser nach außen gegeben wird...
Wenn jemand zweimal den Eintrag n aus dem Vector holen will, geht das nicht mehr. Denn beim ersten mal muss er ihn mit move aus dem Vector holen (kopieren geht ja nicht) und beim zweiten mal ist er leer... (Wahrscheinlich ist dies aber eine neue Frage)

Schlangenmensch

Die Frage die sich bei der Wahl von Smart Pointern stellt ist halt, wofür man sie benutzen will.

Was zum Beispiel bei dir gehen sollte (nicht probiert)

for(auto& abstract : example.Get()){
   abstract->someFunction();
}

Also, wenn du die Daten wirklich nur ausgeben willst, reicht die const reference.

Quaneu

Ich gebe diesen Vector ja nach außen. Damit ein Anwender damit arbeiten kann bzw. die Daten analysieren kann.
Wenn der Anweder jedoch folgendes machen würde:

const std::vector<std::unique_ptr<Abstract>>& items = example.Get();

for(auto& abstract : items ){
   abstract->someFunction();
}

...

for(auto& abstract : items ){
   abstract->someFunction();
}

Würde die zweite Schleife nur noch leere "Hüllen" liefern... Dies finde ich sehr unschön, daher denke ich, dass es so gar keinen Sinn macht.

Quaneu schrieb:

Wenn der Anweder jedoch folgendes machen würde:
const std::vector<std::unique_ptr<Abstract>>& items = example.Get();

for(auto& abstract : items ){
   abstract->someFunction();
}

...

for(auto& abstract : items ){
   abstract->someFunction();
}
Würde die zweite Schleife nur noch leere "Hüllen" liefern...

Das ist falsch.

Dies finde ich sehr unschön, daher denke ich, dass es so gar keinen Sinn macht.

Wenn dem so wäre, wäre es unschön. Es ist aber nicht so. (hast du es ausprobiert?)

firefly

Quaneu schrieb:

Ich gebe diesen Vector ja nach außen. Damit ein Anwender damit arbeiten kann bzw. die Daten analysieren kann.
Wenn der Anweder jedoch folgendes machen würde:
const std::vector<std::unique_ptr<Abstract>>& items = example.Get();

for(auto& abstract : items ){
   abstract->someFunction();
}

...

for(auto& abstract : items ){
   abstract->someFunction();
}
Würde die zweite Schleife nur noch leere "Hüllen" liefern... Dies finde ich sehr unschön, daher denke ich, dass es so gar keinen Sinn macht.

Woher kommt das "wissen", dass die unique_ptrs in items nach der ersten schleife leer sind?
Hast du das ausprobiert oder vermutest du nur?

Im deinem Beispiel ist die schleife das gleiche wie

for(std::unique_ptr<Abstract>& abstract : items ){
   abstract->someFunction();
}

Da wird nichts gemoved.

Quaneu

Entschuldige Du hast recht, das Beispiel war falsch. Hatte es mit items.at() ausprobiert. Und da geht es nicht, wenn man sich zweimal den selben Eintrag holt.

Quaneu schrieb:

Entschuldige Du hast recht, das Beispiel war falsch. Hatte es mit items.at() ausprobiert. Und da geht es nicht, wenn man sich zweimal den selben Eintrag holt.

Auch das ist Blödsinn.

Quaneu

Kommt eben drauf an wie man sich den Eintrag holt.

auto &a = items.at(0); // Geht zweimal
auto a = std::move(items.at(0)); // Geht nicht zweimal

Ich halte es eher so, daß ich versuche, einen unique_ptr gar nicht erst aus meiner Klasse heraus zu geben, außer wenn tatsächlich ein Besitz-Transfer stattfinden soll.

Die Regel wäre etwa folgende:

Der Besitzer des unique_ptr ist ALLEINIGER Besitzer des Objekts
Sollen keine anderen Objekte die betreffenden Objekte besitzen dürfen, wird ein raw-pointer zurückgegeben
raw-pointer sind weak-pointer, garantieren also keinen Besitz über das betreffende Objekt
Sollen andere Objekte ebenfalls Besitz übernehmen können, sollte ein shared_ptr verwendet werden

Hatte allerdings glaube ich noch nicht den Fall, daß ich eine komplette Objektliste aus unique pointern zurückgeben musste.
Theoretisch kann man da eine zweite Liste aus raw-pointern in der Klasse speichern und die dann anstatt der unique_ptr Liste zurückgeben, aber arg schön ist das auch nicht. Da würde ich dann eher zu shared pointern tendieren. Wenn dein Code nicht nur über die Liste iteriert, sondern auch mal ein Objekt aus der Liste irgendwo anders abspeichern will wäre das eh die bessere Wahl.

firefly

Quaneu schrieb:

Kommt eben drauf an wie man sich den Eintrag holt.
auto &a = items.at(0); // Geht zweimal
auto a = std::move(items.at(0)); // Geht nicht zweimal

Wiso willst du das item raus nehmen?
Wenn es nur benötigt wird um daten aus dem element zu lesen reicht es über die referenz da muss nichts kopiert/gemoved werden

Quaneu schrieb:

Kommt eben drauf an wie man sich den Eintrag holt.
auto &a = items.at(0); // Geht zweimal
auto a = std::move(items.at(0)); // Geht nicht zweimal

Unteres kompiliert nicht einmal. Ergo: Blödsinn.

Grund: du hast aus deiner Funktion eine const-ref zurückgegeben.

Außerdem: wenn das jetzt nicht const wäre, hättest du ja explizit mit std::move aufgefordert, den Originalwert zu ändern. Dann wärst du selbst schuld.

Quaneu schrieb:

Mittlerweile zweifel ich jedoch grundsätzlich an der Verwendung von unique_ptr in einem Vector, wenn dieser nach außen gegeben wird...

Einfache Pointer sind da auch nicht unbedingt besser: man kann delete aufrufen oder den Wert ändern.
Getter/Setter werden sowieso viel zu exzessiv verwendet. Man sollte dann aber wenigstens keinen Zugriff auf die Innereien haben (wie in deinem Fall).
Wenn du keine Nullpointer in deinem Vector hast, könnte man da was mit boost löten und Iteratoren auf konstante Referenzen statt einem kompletten Vector ausliefern:

#include <iostream>
#include <vector>
#include <memory>
#include <algorithm>
#include <boost/iterator/indirect_iterator.hpp>

int main()
{
  std::vector<std::unique_ptr<int>> v;
  v.emplace_back(new int{1});
  v.emplace_back(new int{2});

  boost::indirect_iterator<decltype(v.begin()), const int&> 
    indirect_first(v.begin()), indirect_last(v.end());

  std::copy(indirect_first, indirect_last, std::ostream_iterator<int>(std::cout, ","));

  std::cout << std::endl;  
}