Hilfestellung für exception-Sicherheit und RAII

[SBond]

Ich denke die Wartbarkeit und der Lebenszyklus der Software spielen schon eine Rolle
Naja... wenn meine Software wirklich gut sein sollte, dann bräuchte ich noch 1-2 Optimierungsdurchläufe. Ich sehe es an den Bereichen, bei denen ich gerade mit C++ angefangen hatte. Zumindest habe ich in den letzten Monaten einige Fortschritte erziehlt.

Nochmal kurz zu den Tipp mit der eigenen Klasse. Da ist natürlich was dran. Momentan sehe ich zwei Möglichkeiten meine bestehenen Funktionen zu verbessern:

1. mit unique_ptr/shared_ptr:

Header:

static auto X509_closer = [](X509* t) {X509_free(t);};

Cpp:

void meineFunktion ()
{
	unique_ptr<X509, decltype(X509_closer)> myNewCert(X509_new(), X509_closer);
	// ...irgendwelcher code ...
}

2. mit eigener Klasse:

Header:

class X509Guard
{
public:
	X509Guard(X509 *x509)
	{
		m_x509 = x509;
	}
	~X509Guard()
	{
		free();
	}
	void free()
	{
		if (m_x509 != nullptr)
		{
			X509_free(m_x509);
		}
	}

	X509 *get()
	{
		return m_x509;
	}

	void reset(X509 *x509)
	{
		free();
		m_x509 = x509;
	}

	X509 *release()
	{
		X509 *tmp = m_x509;
		m_x509 = nullptr;
		return tmp;
	}

private:
	X509 *m_x509;
};

Cpp:

void meineFunktion ()
{
	X509Guard myNewCert(X509_new());
	// ...irgendwelcher code ...
}

Könnt ihr eine Empfehlung geben, welche Variante die bessere ist? Die eigene Klasse hat zumindest den Vorteil, dass diese besser zu lesen ist. Allerdings ist funktionalität im Vergleich zu unique_ptr geringer bzw. ich müsste diese nachimplementieren. Außerdem müsste ich wohl für jeden Datentyp eine Klasse erstellen, da einige Typen verschiedene deleter haben (z.B. von der Anzahl der Parameter). ...ein Template würde wohl nicht so ohne weiteres gehen.

Oder kann man irgendwie beides Kombinieren? Also ein unique_ptr den ich auch genau so verwende, aber die Initialisierung wie bei meinem 'X509Guard' mache (d.h. die Angabe des deleters wird irgendwie ausgelagert)? ...möglicherwise ginge es über Makros. Aber selbst wenn, wäre es bestimmt keine gute Lösung.

In erster Linie würde ich zu Variante 1 tendieren, da diese etwas weniger Aufwand darstellt und weniger Code benötigt wird. Mein Problem sind Ausdrücke wie diese hier (Code-Auszug):

unique_ptr<ASN1_OCTET_STRING, decltype(ASN1_STRING_delete)> subKeyId(nullptr, ASN1_STRING_delete);
subKeyId.reset(static_cast<ASN1_OCTET_STRING*>(X509_get_ext_d2i(x509 ,nid, nullptr, nullptr)));

Das ist schon in zwei Zeilen aufgebrochen und sieht dennoch unübersichtlich aus. So etwas ist in meinem Code nur 2-3 mal vorhanden, aber ich denke immer, das geht doch bestimmt übersichtlicher.

[Techel]

Mach doch ein Typedef.

[Skym0sh0]

Ich persönlich bevorzuge die Variante mit der eigenen kapselnden Klasse.

Du erhöhst damit ein wenig die Abstraktion und machst dem durch den Code sozusagen eher klar, was deine Entität macht als wenn du sie nur in einen unique_ptr einkapselst. Und du erreichst etwas Flexibilität für später, falls/wenn du mal deine Interna austauschst, sprich OpenSSL::X509 durch ClosedSSL::KeyStuff ersetzen willst.

[SBond]

@Techel: beim Typedef weiß ich gar nicht so recht was ich da machen könnte. Kannst du ein kurzes Beispiel geben, wie so etwas aussehen könnte?

@Skym0sh0: Wäre es sinnvoll ein Klasse zu erstellen, die gleich mehrere Datentypen verwaltet? Was mich ein wenig von der Klasse abschreckt, ist meine aktuelle Vorstellung davon, für jeden Datentyp eine Klasse zu erstellen. Sollte ich später mal Änderungen machen (z.B. erweitern), dann müsste ich jede Klasse anpassen.

Es gibt ja dieses 'typeid(meineVariable).name()', das den Typ einer Variable identifiziert. Eventuell könnte man irgendwie den gelieferten Pointer (über den Konstruktor) in ein void* packen und den Datentyp als std::string speichern. Beim Löschen wird dann geschaut, welcher Datantyp es ist. ...oder das ist völliger Blödsinn. ...ist so eine Idee

[Techel]

Na zum Beispiel soetwas:

typedef unique_ptr<ASN1_OCTET_STRING, decltype(ASN1_STRING_delete)> X509Guard

Da hast du dann im Grunde das gleiche wie wenn du eine neue Klasse erstellst.

Deine letzte Idee ist tatsächlich Blödsinn, das fügt nur unnötigen Overhead hinzu

[SBond]

ach mensch natürlich

was würde ich nur ohne euch machen. Wenn ihr meine Nachbarn wärt, dann würde ich euch zum Grillen und eine Flasche Bier einladen

[hustbaer]

Ich verwende bei sowas z.T. auch einfach shared_ptr .
Der Vorteil bei shared_ptr ist dass der Typ des Deleters sich nicht im Smart-Pointer-Typ niederschlägt.
Damit kann man dann schön Factory-Funktionen machen.
Die Factory-Funktionen kann man dann verwenden ohne sich darum kümmern zu müssen wie das Objekt jetzt wirklich gelöscht wird.
Ist vielleicht nicht die "schönste" Lösung (weil man shared-Ownership auch dort ermöglicht wo man es gar nicht braucht, und natürlich den shared_ptr Overhead dadruch auch unnötig mitschleppt). Aber für viele Anwendungen vollkommen ausreichend.

Ansonsten, wenn dein Deleter Default-Constructible ist, dann kannst du natürlich auch einen unique_ptr typedef verwenden. Dann musst du auch nichts unnötig wiederholen. Durch den typedef vermeidest du den Deleter-Typ überall zu wiederholen und dadurch dass er Default-Constructible ist vermeidest du die Konstruktor-Parameter für den Deleter überall zu wiederholen.

[SBond]

Ja stimmt

Somit hätte ich wohl folgende Varianten:

static auto X509_deleter = [](X509* t) {X509_free(t);};
typedef std::unique_ptr<X509, decltype(X509_deleter)> X509_Guard;

// unique_ptr ohne typedef
std::unique_ptr<X509, decltype(X509_deleter)> myVar1(nullptr, X509_deleter);

// unique_ptr mit typedef
X509_Guard myVar2(nullptr, X509_deleter);

// shared_ptr
std::shared_ptr<X509> myVar3(nullptr, X509_free);

Noch mal eine Frage am Rande. Kann ich sowas problemlos machen?:

std::shared_ptr<X509> meineFunktion()
{
    std::unique_ptr<X509, decltype(X509_deleter)> myVar1(X509_new(), X509_deleter);

    return myVar1.release();
}

std::shared_ptr<X509> meineFunktion()
{
    std::shared_ptr<X509> myVar1(X509_new(), X509_free);

    // Variante 1
    return myVar1.release();

    // Variante 2
    return myVar1;
}

... für den Fall, dass ich einen Pointer teilen will
(habe es gerade noch nicht getestet)

[hustbaer]

SBond schrieb:

Noch mal eine Frage am Rande. Kann ich sowas problemlos machen?:

std::shared_ptr<X509> meineFunktion()
{
    std::unique_ptr<X509, decltype(X509_deleter)> myVar1(X509_new(), X509_deleter);

    return myVar1.release();
}

Nein.
Problem #1 Der Deleter wird nicht automatisch mit übernommen. Du musst ihn beim Erzeugen des shared_ptr nochmal angeben.
Problem #2 Die Konstruktoren von shared_ptr die einen T* nehmen sind explicit .

So sollte es gehen

std::shared_ptr<X509> meineFunktion()
{
    std::unique_ptr<X509, decltype(X509_deleter)> myVar1(X509_new(), X509_deleter);

    return std::shared_ptr<X509>(myVar1.release(), myVar1.get_deleter());
// Oder auch einfach
    return move(myVar1);
}

SBond schrieb:

std::shared_ptr<X509> meineFunktion()
{
    std::shared_ptr<X509> myVar1(X509_new(), X509_free);

    // Variante 1
    return myVar1.release();

    // Variante 2
    return myVar1;
}

Variante 1: Nein. shared_ptr hat keine Release Funktion. Darf auch keine haben.

Variante 2: Ja, das geht.

[hustbaer]

Ich möchte aber nochmal auf die Sache mit default konstruierbaren Deletern hinweisen:

struct X509_deleter
{
    void operator()(X509* p)
    {
        if (p)
            X509_free(p);
    }
};

typedef std::unique_ptr<X509, X509_deleter> X509_Guard;

void Foo()
{
    X509_Guard g(X509_new());
    // ...
}

Ist mMn. viel angenehmer zu verwenden.
Ich mag auf jeden Fall keinen Code wo ich jedes mal wo ich ein Objekt erstelle noch unnötigerweise mit angeben muss wie es gelöscht werden soll.

[oops]

Meiner Meinung sollte die Klasse X509 heissen, das X509_new() gehört in den Konstruktor und die Klasse sollte alle fachlichen Methoden haben um mit dem Objekt zu arbeiten.

[SBond]

SBond schrieb:

Ja stimmt

Noch mal eine Frage am Rande. Kann ich sowas problemlos machen?:

std::shared_ptr<X509> meineFunktion()
{
    std::unique_ptr<X509, decltype(X509_deleter)> myVar1(X509_new(), X509_deleter);

    return myVar1.release();
}

std::shared_ptr<X509> meineFunktion()
{
    std::shared_ptr<X509> myVar1(X509_new(), X509_free);

    // Variante 1
    return myVar1.release();

    // Variante 2
    return myVar1;
}

Ja das war wiklich Blödsinn von mir. Aber der Compiler klopft einen da ja auf die Finger :). Tatsächlich ist der move()-Befehl hier die richtige Wahl. Hatte vor einiger Zeit etwas darüber gelesen, aber da ich noch keine Anwendung fand, geriet es wieder in Vergessenheit. Auch nochmal vielen Dank für den Tipp mit der ()-Überladung. Das ist eine echt gute Ergänzung

nochmals vielen Dank für die tolle Unterstützung von euch
Damit lässt sich der Quellcode viel besser exception-sicherer gestalten, als meine ersten Versuche mit try/catch in jeder Funktion. *freu*