Hilfestellung für exception-Sicherheit und RAII


  • Mod

    SBond schrieb:

    static auto TYPE_closer = [](TYPE* t) { TYPE_free(t); };
    

    Warum eigentlich static? Ohne funktioniert es scheinbar auch.

    Weil ich mir dachte, dass der wahrscheinlichste Fall ist, dass dies global in einer Implementierungsdatei stehen wird, in der du das brauchst. Und da dann eben static, um das Ding nicht unnötig überall sichtbar zu haben.

    unique_ptr<TYPE, decltype(TYPE_closer)> meineVariable(eineOpenSslFunktion(...), TYPE_closer);
    

    Kann man das auch in zwei Zeilen zerlegen?

    Sollte machbar sein, wenn man ein paar Hürden überspringt, aber wieso das nicht sinnvoll ist, hat manni66 erklärt. Du willst ja gerade von dieser Art der Programmierung weg.



  • SBond schrieb:

    TYPE ist irgendein Datentyp. Im Falle OpenSSL gibt es diese zu Hauf. Jeder Datentyp hat da seine eigene Aufräumfunktion (teilweise unterschiedlich von den Parametern). Wie kann ich sowas am besten mit RAII umsetzen? Muss ich für jeden Datentyp extra eine ScopeGuard-Klasse definieren? Oder kann man einen allgemeinen ScopeGuard erstellen, bei dem man als Parameter angibt, wie die Ressource freigegeben wird.

    Machen kannst du beides. Wobei die Variante für jeden Datentyp eine eigene Klasse zu basteln langfristig meist angenehmer ist. Zumindest wenn man jeden Datentyp mehr als 1-2 mal benötigt.
    Bei generischen Scope-Guards (scoped_ptr, shared_ptr etc.) musst du die Cleanup-Funktion an jeder Stelle wo du einen bestimmten Datentyp brauchst immer wiederholen.



  • vielen Dank für die Tipps. 😃

    Wahrscheinlich ist es wirklich sinnvoller die Variablen nur dort zu deklarieren/initialisieren, wo sie erst benötigt werden. Bisher war ich es immer anders gewohnt. Ich hatte es vor einiger Zeit mal aus einem Coding Guideline übernommen. Ich wollte diesen Stil in meiner jetzigen Programmierung nicht mehr ändern, da es bei ca 35K Programmzeilen schon einen gewissen Aufwand darstellt, dies global anzupassen (ich möchte gerne einen konsistenten Quellcode haben).

    Dank eurer Hilfe konnte ich aber schon einiges verbessern 🙂

    Zum Thema RAII muss ich mich wohl noch etwas besser belesen. Ich halte es schon für wichtig (insbesondere bei Verwendung von exceptions). Viele größere Code-Beispiele verwirren mich noch etwas, aber das wird schon.



  • SeppJ schrieb:

    SBond schrieb:

    static auto TYPE_closer = [](TYPE* t) { TYPE_free(t); };
    

    Warum eigentlich static? Ohne funktioniert es scheinbar auch.

    Weil ich mir dachte, dass der wahrscheinlichste Fall ist, dass dies global in einer Implementierungsdatei stehen wird, in der du das brauchst. Und da dann eben static, um das Ding nicht unnötig überall sichtbar zu haben.

    unique_ptr<TYPE, decltype(TYPE_closer)> meineVariable(eineOpenSslFunktion(...), TYPE_closer);
    

    Kann man das auch in zwei Zeilen zerlegen?

    Sollte machbar sein, wenn man ein paar Hürden überspringt, aber wieso das nicht sinnvoll ist, hat manni66 erklärt. Du willst ja gerade von dieser Art der Programmierung weg.

    Hat alles super funktioniert! Vielen Dank 😃

    nochmal zur Trennung in 2 Zeilen. Die Lösung war ja doch recht einfach:

    unique_ptr<TYPE, decltype(TYPE_closer)> meineVariable(nullptr, TYPE_closer);
    //......
    meineVariable.reset(eineOpenSslFunktion(...));
    

    (ist nur noch mla zur Ergänzung)



  • Ich wollte diesen Stil in meiner jetzigen Programmierung nicht mehr ändern, da es bei ca 35K Programmzeilen schon einen gewissen Aufwand darstellt, dies global anzupassen (ich möchte gerne einen konsistenten Quellcode haben).

    Kann ich irgendwo verstehen. Andrerseits sollte man genau diesen Grund immer hinterfragen. Speziell wenn klar ist wie die bessere Variante aussieht, und es nicht 100 verschiedene Arten es zu machen gibt, sondern nur die schlechte und die bessere, sollte man nochmal drüber nachdenken ob es nicht doch besser wäre mal etwas "Uneinheitlichkeit" zu akzeptieren.
    Neue Funktionen nach dem besseren Muster zu stricken. Und alle Teile wo man mehr als nur 1-2 Zeilen anpasst bei der nächsten Änderung einfach umstellt. Ich mache das in einem Programm das ich "geerbt" habe schon die letzten paar Jahre, und es hat mir das Leben durchaus deutlich erleichtert.
    Einige Teile sind immer noch ein unübersichtlicher furchtbarer sauhaufen, aber andere sind dadurch mittlerweile fast schon "gut wartbar" geworden.

    Kommt aber natürlich auch drauf an wie lange du das Programm noch weiterentwickeln und/oder pflegen musst. Wenn du das in ein paar Monaten nicht mehr anfassen musst, dann ist es vielleicht wirklich OK es einfach mal so zu lassen.



  • 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.



  • Mach doch ein Typedef.



  • 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.



  • @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



  • 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 😉



  • 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 😃



  • 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.



  • 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)



  • 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.



  • 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.



  • 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 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*


Anmelden zum Antworten