Kopierkonstruktor ohne Speicher freigeben?

Sorry, dachte ich könnte mal eben nen Beispiel aus dem Kopf tippen
War wohl daneben

Ich demonstriere einfach mal mein problem. Ich wollte einen Wrapper (immer noch zum Üben und Rumprobieren) für die pcap-Library basteln.
Um die Devices anzuzeigen, wollte ich einen Vektor a la CDevice (eine Klasse von mir) benutzen.

int CPcap::ListAllDevices(std::vector<CDevice>& aDevices) {
	int ret_finddevs;
	pcap_if_t *devices;
	//============================================>
	aDevices.clear();
	ret_finddevs = pcap_findalldevs(&devices, _errorbuf);
	if (ret_finddevs == 0) {
		for (pcap_if_t* myDevice = devices;myDevice;myDevice = myDevice->next) {
			aDevices.push_back(myDevice);
		}
		pcap_freealldevs(devices);
	}
	return ret_finddevs;
}

Diese Funktion liest die Devices ein und speichert sie im Vektor ab.
Hier ist die Klasse CDevice:

#pragma once

#include "Address.h"
#include <pcap.h>
#include <vector>
#include <iostream>

class CDevice
{
public:
	//Methoden
	CDevice(const pcap_if_t *myDevice);
	~CDevice(void);
	CDevice(const CDevice& rhs);
	//Elemente
	char *name, *description;
	u_int flags;
	std::vector<CAddress> aAddresses; //kümmern wir uns später drum
private:
	//Methoden
	//Elemente
};

#include "Device.h"

CDevice::CDevice(const pcap_if_t *myDevice) //Konstruktor
{
	//====================================>
	name = new char[strlen(myDevice->name)+1];
	memcpy(name, myDevice->name, strlen(myDevice->name)+1);
	description = new char[strlen(myDevice->description)+1];
	memcpy(description, myDevice->description, strlen(myDevice->description)+1);
	flags = myDevice->flags;
}

CDevice::~CDevice(void)
{
	delete[] name;
	delete[] description;
}

CDevice::CDevice(const CDevice& rhs) {
	name = new char[strlen(rhs.name)+1];
	strcpy(name, rhs.name);
	description = new char[strlen(rhs.description)+1];
	strcpy(description, rhs.description);
	flags = rhs.flags;

}

Der Code funktioniert zwar, aber ich verstehe nicht, warum ich da einen Kopierkonstruktor brauche (und ja, ich habe den Code selber geschrieben. Google meinte, dass man für push_back einen Kopierkonstruktor braucht).

Ich möchte in diesem Fall nur ungern std::string benutzen, da die Klasse kompatibel zu den Funktionen der pcap-Library bleiben soll. Man könnte hier bestimmt auch std::string verwenden, will ich aber gerade nicht (ich will verstehen, warum ich hier einen Kopierkonstruktor brauche).

daddy_felix

Der Kopierkonstruktor wird genutzt, wenn Objekte per push_back hinzugefügt werden. Ein Kopierkonstruktor wird automatisch erzeugt, nur macht dieser nicht immer das, was du möchtest. Wenn du einen Pointer als Member hast, wird dieser Pointer vom automatisch generierten Kopierkonstruktor kopiert - nicht aber das Objekt, auf das der Pointer zeigt.

Wenn du im Konstruktor den Speicherplatz mit "new" erzeugst und anschließend ein neues Objekt per Kopierkonstruktor erstellst, hast du zwei Pointer, die auf dasselbe Objekt zeigen. Wenn nun ein Objekt zerstört wird, dann ruft der Destruktor (hoffentlich!) "delete" auf - und der zweite Pointer zeigt ins Nirvana.

Deshalb musst du einen eigenen Kopierkonstruktor schreiben, der nicht nur den Pointer kopiert, sondern das referenzierte Objekt. Gleiches gilt für den Zuweisungsoperator - google mal nach "Regel der großen Drei".

SeppJ

Den Kopierkonstruktor brauchst du, weil vector den Kopierkonstruktor benutzt, um Elemente zu, äh, kopieren.

Ein push_back ist keine Zuweisung. Dazu müsste dann ja erst ein Leerelement erzeut werden (und wie sähe dieses aus?), welches sofort überschrieben wird. Es ist auch (zumindest im Allgemeinen) keine Verschiebung, da sonst schließlich das Original hinterher weg wäre.

new[] ist niemals eine bessere Alternative zu vector oder string. Wenn du dich über mangelnde Kompatibilität des Interfaces beschwerst, dann kennst du anscheinend das interface von vector und string nicht gut genug. Dieses ist kompatibel zu den Arrays im C-Stil:
String:
http://www.cplusplus.com/reference/string/string/c_str/
http://www.cplusplus.com/reference/string/string/data/
Achtung, der Bereich den data und c_str zurück geben, ist const!
Vector:
http://www.cplusplus.com/reference/stl/vector/operator[]/
Es ist garantiert, dass die Elemente hintereinander und dicht im Speicher liegen, d.h. &charvector[0] entspricht einem Zeiger auf den Anfang eines char-Arrays. Und der Bereich ist nicht const! Oder in C++11 in Kurzschreibweise:
http://en.cppreference.com/w/cpp/container/vector/data
Bei vector ist data nicht const!

Ich wollte eigentlich wissen, warum push_back nicht einfach ein neues Objekt erzeugt... Dann könnte er sich den Kopierkonstruktor sparen. Wenn ich das richtig verstanden habe, erzeugt push_back ein neues Objekt mit dem Kontruktorargument und kopiert es anschliessend zu sich selbst... Warum erstellt er nicht einfach ein neues Objekt und lässt das Kopieren weg?

krümelkacker

Ordne eine Ressource sofort einem Objekt zu, so lokal wie möglich. Und lass ein Objekt nach Möglichkeit nicht mehrere Ressourcen direkt/manuell verwalten.

ListAllDerives ist nicht Ausnahme-sicher, da Du manuell pcap_freealldevs aufrufen musst, es aber vorher zu einer Ausnahme kommen könnte.

name/description in CDevice sind "owning raw pointers". Pfui. Du kannst dir viel Ärger ersparen, indem Du die Ressourcenverwaltung von CDevice für mehrere Ressourcen (generell schlecht, nimm ein Objekt pro Ressource) eine Ebene tiefer verlagerst: std::string statt char* . Dann kannst du dir auch sparen, Kopierkonstruktor, Zuweisungsoperator und Destruktor von CDevice selbst zu definieren.

Warum kompliziert wenn es auch einfach geht?

Schaff dir ein gescheites Buch an, was dir C++ richtig beibringt.

Telefonmann schrieb:

Ich möchte in diesem Fall nur ungern std::string benutzen,

Telefonmann schrieb:

ich will verstehen, warum ich hier einen Kopierkonstruktor brauche

Was ist das denn für eine Aussage?
Ein Kopierkonstruktor ist immer dann nötig, wenn du ein neues Objekt als Kopie erzeugen willst. Ein Zuweisungsoperator ist immer dann nötig, wenn du einem Objekt den "Wert" eines anderen zuweisen willst. Klar oder?

daddy_felix

Telefonmann schrieb:

Ich wollte eigentlich wissen, warum push_back nicht einfach ein neues Objekt erzeugt...

tut es doch.

Telefonmann schrieb:

Wenn ich das richtig verstanden habe, erzeugt push_back ein neues Objekt mit dem Kontruktorargument und kopiert es anschliessend zu sich selbst...

Das hast du falsch verstanden. Schau dir doch den Kopierkonstruktor nochmal genau an. Dieser funktioniert so:

XXX defaultObject(); // default constructor
XXX argumentObject("Hello"); // constructor with arguments

XXX copyObject(defaultObject); // copy constructor
XXX assignment = argumentObject; // assignment operator

krümelkacker

daddy_felix schrieb:

XXX defaultObject(); // default constructor

Du meinst wahrscheinlich // Funktionsdeklaration ...

daddy_felix

krümelkacker schrieb:

daddy_felix schrieb:
XXX defaultObject(); // default constructor
Du meinst wahrscheinlich // Funktionsdeklaration ...

Bei dir ist der Name aber auch Programm Das kommt davon, wenn man gerade drei Wochen Urlaub hatte...

XXX defaultObject; // default constructor

ipsec

Telefonmann schrieb:

Wenn ich das richtig verstanden habe, erzeugt push_back ein neues Objekt mit dem Kontruktorargument und kopiert es anschliessend zu sich selbst

push_back übernimmt als Parameter nicht den Konstruktorparameter für das neue Objekt, sondern ein vorhandenes Objekt, welches dann in den Vektor kopiert wird. An dieser Stelle tritt die Kopie auf. In deinem Fall funktioniert das trotzdem, weil CDevice einen nicht- explicit Konstruktor besitzt, der ein pcap_if_t* als Parameter besitzt, mit diesem kann der Compiler an der Stelle aDevices.push_back(myDevice) ein temporäres CDevice-Objekt erzeugen, welches dann push_back als Parameter übergeben wird.
Seit C++11 unterstützt vector auf das Einfügen mit direkter Übergabe der Konstruktorparameter für das neu zu erstellende Objekt, womit diese Kopie wegfallen kann. Die Methode dazu heißt emplace_back .

Generell sollte man aber immer einen funktionierenden Kopierkonstruktor implementieren (oder diesen zur Not unzugänglich machen), da von C++-Objekten funktionierende Kopiersemantik erwartet wird. Noch besser ist es aber, die Resourcen (insbesondere die Pointer) nicht selbst zu verwalten, dann machen die compilergenerierten Kopierkonstruktoren und Zuweisungsoperatoren schon das richtige und man muss sich darum überhaupt nicht kümmern.

@ipsec Jaaa, genau das meinte ich.

Nur folgendes verstehe ich nicht ganz:

Ordne eine Ressource sofort einem Objekt zu, so lokal wie möglich.
...
ListAllDerives ist nicht Ausnahme-sicher, da Du manuell pcap_freealldevs aufrufen musst, es aber vorher zu einer Ausnahme kommen könnte.

Zu 1) Warum sollte ich die Elemente lokal initialisieren? (schon wieder was falsch verstanden?)

Zu 2) Meinst du eine potentielle Ausnahme bei:

aDevices.clear();

?

Ramanujan

SeppJ schrieb:

new[] ist niemals eine bessere Alternative zu vector oder string.

Wirklich niemals? Ich könnte mir Fälle vorstellen, in denen man sehr viele sehr kleine Arrays dynamischer, implizit gegebener Größe benötigt, bei denen der Overhead von vector zu viel Speicher verbrauchen würde. Von daher würde ich sagen, dass new[] fast nie eine bessere Alternative zu vector ist.

Skym0sh0

und selbst dann gibt es immer ncoh sowas wie smart pointer auf arrays, shared_array oder so von boost ist das dann...

und einfach mal so eine ganze reihe kleiner arrays erstellen hört sich auch nicht ganz sauber an, denke da gibt es dan im konkreten fall bessere alternativen (und sau es um "nur" etwaige ausnahmesicherheiten zu garantieren)

krümelkacker

Telefonmann schrieb:

Nur folgendes verstehe ich nicht ganz:

Ordne eine Ressource sofort einem Objekt zu, so lokal wie möglich.
...
ListAllDerives ist nicht Ausnahme-sicher, da Du manuell pcap_freealldevs aufrufen musst, es aber vorher zu einer Ausnahme kommen könnte.

Zu 1) Warum sollte ich die Elemente lokal initialisieren? (schon wieder was falsch verstanden?)

Ich habe mich wahrscheinlich nur blöd ausgedrückt, hatte aber versucht das noch weiter zu beschreiben. Mal an einfachen Beispielen:

doof: (klasse person muss sich um mehrere Ressourcen selbst kümmern)

class person
{
  char* vorname;
  char* nachname;
public:
  person(const char* vn, const char* nn);
  ~person();
  person(person const&);
  person& operator=(person const&);
};

schlau: (klasse person kümmert sich um nix. mystring um nur jeweils genau eine Ressource)

class mystring
{
  char* data;
public:
  ~mystring()
  mystring(mystring const&);
  mystring& operator=(mystring const&);
  ...
};

class person
{
  mystring vorname;
  mystring nachname;
public:
  person(const char* vn, const char* nn);
};

Das meinte ich mit "lokal" ... ist vielleicht nicht das beste Wort dafür, aber ich hoffe, du weißt, wie das gemeint ist.

... und statt mystring nimmste einfach std::string aus dem <string>-Header.

Telefonmann schrieb:

Zu 2) Meinst du eine potentielle Ausnahme bei:
aDevices.clear();
?

Nee, eher bei den push_backs, die du da machst. Gewöhne dir einfach ab, Ressourcen zu "acquirieren" (Speicher, Datei handles, Locks, etc), ohne dabei die Verantwortung bzgl der Freigabe an ein Objekt abzutreten. Wenn du meinst, Kopierkonstruktor, Zuweisungsoperator und Destrkutor von sehr vielen deiner Klassen selbst definieren zu müssen, hast du echt etwas falsch gemacht, siehe person-Klassenbeispiel.

Ah, ok. Habs verstanden. Danke dir

Ramanujan schrieb:

SeppJ schrieb:

new[] ist niemals eine bessere Alternative zu vector oder string.

Wirklich niemals? Ich könnte mir Fälle vorstellen, in denen man sehr viele sehr kleine Arrays dynamischer, implizit gegebener Größe benötigt, bei denen der Overhead von vector zu viel Speicher verbrauchen würde. Von daher würde ich sagen, dass new[] fast nie eine bessere Alternative zu vector ist.

Welchen Overhead gegenüber new[] hat std::vector denn?

SeppJ

manni66 schrieb:

Ramanujan schrieb:

SeppJ schrieb:

new[] ist niemals eine bessere Alternative zu vector oder string.

Wirklich niemals? Ich könnte mir Fälle vorstellen, in denen man sehr viele sehr kleine Arrays dynamischer, implizit gegebener Größe benötigt, bei denen der Overhead von vector zu viel Speicher verbrauchen würde. Von daher würde ich sagen, dass new[] fast nie eine bessere Alternative zu vector ist.

Welchen Overhead gegenüber new[] hat std::vector denn?

Praktisch hat vector in der Regel die Größe von 3 Pointern, wohingegen bei new ein Pointer und eine Größenangabe (~eine weitere Pointergröße) reicht, sofern man nicht vor hat, das Array zu vergrößern.

Das Beispiel ist derart konstruiert, dass ich bei nie bleibe. Wenn wir wirklich immer alle möglichen Ausnahmen berücksichtigen, dann kann man in diesem Forum gar nichts mehr sagen, ohne immer ein "fast" in die Aussage aufzunehmen.

hustbaer

SeppJ schrieb:

Wenn wir wirklich immer alle möglichen Ausnahmen berücksichtigen, dann kann man in diesem Forum gar nichts mehr sagen, ohne immer ein "fast" in die Aussage aufzunehmen.

Man sollte auch fast nie nie schreiben, sondern fast immer fast nie

Bitte tötet mich nicht bei meinen ganzen blöden Fragen ...

Ich versuche gerade eine Klasse in eine andere Klasse einzukapseln. Das problem ist nur, dass die Klasse CEthernet umbedingt Parameter für den Konstruktor braucht. Momentan habe ich dies mit einem Pointer gelöst. Kann man das auch ohne Pointer lösen oder muss das so sein? Ist das in diesem Fall in Ordnung oder sollte man da lieber eine extra Methode zum Einlesen schreiben? (Ich meine soetwas ähnliches schonmal bei QT gesehen zu haben)

class CPacket
{
public:
	//Methoden
	CPacket(const struct pcap_pkthdr *pkt_header, const u_char *pkt_data);
	~CPacket(void);
	//Elemente
	CEthernet *eth;

private:
};

class CEthernet
{
public:
	//Methoden
	CEthernet(const struct pcap_pkthdr *pkt_header, const u_char *pkt_data);
	~CEthernet(void);
	//Elemente
	u_char dstmac[6], srcmac[6];
	short frametype;
};

...

CPacket::CPacket(const struct pcap_pkthdr *pkt_header, const u_char *pkt_data)
{
	//======================================================>
	//Ethernet-Schicht einlesen (14 Bytes)
	eth = new CEthernet(pkt_header, pkt_data); //unschön?
}

CPacket::~CPacket(void)
{
	delete eth;
}

Kellerautomat

C-Präfixe kotzen mich an.

TyRoXx

Telefonmann schrieb:

Ich versuche gerade eine Klasse in eine andere Klasse einzukapseln. Das problem ist nur, dass die Klasse CEthernet umbedingt Parameter für den Konstruktor braucht.

Initialisierungsliste