Warum ein undurchsichtiges „Handle“ verwenden, für das ein Casting in einer öffentlichen API anstelle eines typsicheren Strukturzeigers erforderlich ist?

Ich bewerte eine Bibliothek, deren öffentliche API derzeit so aussieht:

libengine.h

/* Handle, used for all APIs */
typedef size_t enh;


/* Create new engine instance; result returned in handle */
int en_open(int mode, enh *handle);

/* Start an engine */
int en_start(enh handle);

/* Add a new hook to the engine; hook handle returned in h2 */
int en_add_hook(enh handle, int hooknum, enh *h2);

Beachten Sie, dass enhes sich um ein generisches Handle handelt, das als Handle für verschiedene Datentypen ( Engines und Hooks ) verwendet wird.

Intern wandeln die meisten dieser APIs das "Handle" natürlich in eine interne Struktur um, die sie haben malloc:

engine.c

struct engine
{
    // ... implementation details ...
};

int en_open(int mode, *enh handle)
{
    struct engine *en;

    en = malloc(sizeof(*en));
    if (!en)
        return -1;

    // ...initialization...

    *handle = (enh)en;
    return 0;
}

int en_start(enh handle)
{
    struct engine *en = (struct engine*)handle;

    return en->start(en);
}

Persönlich hasse ich es, Dinge hinter typedefs zu verstecken , besonders wenn es die Typensicherheit gefährdet. (Angenommen enh, woher weiß ich, worauf es sich tatsächlich bezieht?)

Daher habe ich eine Pull-Anfrage gesendet, in der die folgende API-Änderung vorgeschlagen wird (nachdem die gesamte Bibliothek entsprechend geändert wurde ):

libengine.h

struct engine;           /* Forward declaration */
typedef size_t hook_h;    /* Still a handle, for other reasons */


/* Create new engine instance, result returned in en */
int en_open(int mode, struct engine **en);

/* Start an engine */
int en_start(struct engine *en);

/* Add a new hook to the engine; hook handle returned in hh */
int en_add_hook(struct engine *en, int hooknum, hook_h *hh);

Dadurch sehen die internen API-Implementierungen natürlich viel besser aus, es werden keine Umsetzungen vorgenommen, und die Typensicherheit wird aus Sicht des Verbrauchers gewahrt.

libengine.c

struct engine
{
    // ... implementation details ...
};

int en_open(int mode, struct engine **en)
{
    struct engine *_e;

    _e = malloc(sizeof(*_e));
    if (!_e)
        return -1;

    // ...initialization...

    *en = _e;
    return 0;
}

int en_start(struct engine *en)
{
    return en->start(en);
}

Ich bevorzuge dies aus folgenden Gründen:

• Typensicherheit hinzugefügt
• Verbesserte Klarheit der Typen und ihres Zwecks
• Besetzungen und typedefs entfernt
• Es folgt dem empfohlenen Muster für opake Typen in C

Der Eigentümer des Projekts wies jedoch die Pull-Anfrage zurück (umschrieben):

Persönlich mag ich die Idee nicht, das zu entlarven struct engine. Ich denke immer noch, dass der derzeitige Weg sauberer und freundlicher ist.

Ursprünglich habe ich einen anderen Datentyp für das Hook-Handle verwendet, mich dann aber für die Verwendung entschieden enh, sodass alle Arten von Handles denselben Datentyp verwenden, um es einfach zu halten. Wenn dies verwirrend ist, können wir sicherlich einen anderen Datentyp verwenden.

Mal sehen, was andere über diese PR denken.

Diese Bibliothek befindet sich derzeit in einer privaten Beta-Phase, daher gibt es (noch) nicht viel zu befürchten. Außerdem habe ich die Namen etwas verschleiert.

Wie ist ein undurchsichtiger Griff besser als eine benannte undurchsichtige Struktur?

_{Hinweis: Ich habe diese Frage bei Code Review gestellt , wo sie geschlossen wurde.}

Das Mantra "Einfach ist besser" ist zu dogmatisch geworden. Einfach ist nicht immer besser, wenn es andere Dinge kompliziert. Assemblierung ist einfach - jeder Befehl ist viel einfacher als Sprachbefehle höherer Ebenen - und dennoch sind Assemblierungsprogramme komplexer als Sprachen höherer Ebenen, die dasselbe tun. In Ihrem Fall enhvereinfacht der einheitliche Grifftyp die Typen auf Kosten der Komplexisierung der Funktionen. Da in der Regel die Typen von Projekten im Vergleich zu ihren Funktionen sublinear zunehmen, bevorzugen Sie in der Regel komplexere Typen, wenn dies die Funktionen vereinfacht. In dieser Hinsicht scheint Ihr Ansatz der richtige zu sein.

Der Autor des Projekts ist besorgt darüber, dass Ihr Ansatz darin besteht, das " bloßzustellenstruct engine ". Ich hätte ihnen erklärt, dass es nicht die Struktur selbst verfügbar macht - nur die Tatsache, dass es eine benannte Struktur gibt engine. Der Benutzer der Bibliothek muss diesen Typ bereits kennen - er muss beispielsweise wissen, dass das erste Argument en_add_hookdieses Typs und das erste Argument eines anderen Typs ist. Dies macht die API tatsächlich komplexer, da diese Typen nicht mehr durch die "Signatur" der Funktion dokumentiert werden müssen, sondern an einer anderen Stelle dokumentiert werden müssen und der Compiler die Typen für den Programmierer nicht mehr überprüfen kann.

Eine Sache, die beachtet werden sollte - Ihre neue API macht den Benutzercode etwas komplexer, da statt zu schreiben:

enh en;
en_open(ENGINE_MODE_1, &en);

Sie benötigen jetzt eine komplexere Syntax, um ihr Handle zu deklarieren:

struct engine* en;
en_open(ENGINE_MODE_1, &en);

Die Lösung ist jedoch ganz einfach:

struct _engine;
typedef struct _engine* engine

und jetzt können Sie direkt schreiben:

engine en;
en_open(ENGINE_MODE_1, &en);

Hier scheint es auf beiden Seiten Verwirrung zu geben:

• Die Verwendung eines Handle-Ansatzes erfordert nicht die Verwendung eines einzigen Handle-Typs für alle Handles
• Durch das Anzeigen des structNamens werden seine Details nicht angezeigt (nur seine Existenz).

Die Verwendung von Handles anstelle von bloßen Zeigern in einer Sprache wie C bietet Vorteile, da die Übergabe des Zeigers eine direkte Manipulation des Pointees (einschließlich Aufrufe von free) ermöglicht, während die Übergabe eines Handles erfordert, dass der Client die API durchläuft, um eine Aktion auszuführen .

Der Ansatz, einen einzigen Typ von Handle zu haben, der über a definiert wird, typedefist jedoch nicht typsicher und kann viele Probleme verursachen.

Mein persönlicher Vorschlag wäre daher, mich in Richtung typsicherer Griffe zu bewegen, von denen ich denke, dass Sie beide zufrieden sind. Das geht ganz einfach:

typedef struct {
    size_t id;
} enh;

typedef struct {
    size_t id;
} oth;

Nun kann man nicht versehentlich 2als Griff übergehen , noch kann man einen Griff versehentlich an einen Besenstiel übergeben, wo ein Griff zum Motor erwartet wird.

Ich reichte also eine Pull-Anfrage ein, in der die folgende API-Änderung vorgeschlagen wurde (nachdem die gesamte Bibliothek entsprechend geändert wurde ).

Das ist Ihr Fehler: Bevor Sie umfangreiche Arbeiten an einer Open-Source-Bibliothek durchführen, wenden Sie sich an den / die Autor (en) / Betreuer (en), um die Änderung vorab zu besprechen . Auf diese Weise können Sie beide festlegen, was zu tun ist (oder was nicht), und unnötige Arbeit und die daraus resultierende Frustration vermeiden.

In dieser Situation ist ein undurchsichtiger Griff erforderlich.

struct SimpleEngine {
    int type;  // always SimpleEngine.type = 1
    int a;
};

struct ComplexEngine {
    int type;  // always ComplexEngine.type = 2
    int a, b, c;
};

int en_start(enh handle) {
    switch(*(int*)handle) {
    case 1:
        // treat handle as SimpleEngine
        return start_simple_engine(handle);
    case 2:
        // treat handle as ComplexEngine
        return start_complex_engine(handle);
    }
}

Wenn die Bibliothek über zwei oder mehr Strukturtypen verfügt, die denselben Header-Teil von Feldern aufweisen, z. B. "type", können diese Strukturtypen als übergeordnete Strukturtypen betrachtet werden (z. B. eine Basisklasse in C ++).

Sie können den Header-Teil wie folgt als "struct engine" definieren.

struct engine {
    int type;
};

struct SimpleEngine {
    struct engine base;
    int a;
};

struct ComplexEngine {
    struct engine base;
    int a, b, c;
};

int en_start(struct engine *en) { ... }

Dies ist jedoch eine optionale Entscheidung, da Typumwandlungen unabhängig von der Verwendung der struct engine erforderlich sind.

Fazit

In einigen Fällen gibt es Gründe, warum undurchsichtige Ziehpunkte anstelle von undurchsichtigen benannten Strukturen verwendet werden.

Der offensichtlichste Vorteil des Handles-Ansatzes besteht darin, dass Sie die internen Strukturen ändern können, ohne die externe API zu beschädigen. Zugegeben, Sie müssen die Client-Software noch ändern, aber zumindest ändern Sie nicht die Benutzeroberfläche.

Die andere Möglichkeit besteht darin, zur Laufzeit aus vielen verschiedenen möglichen Typen zu wählen, ohne dass für jeden eine explizite API-Schnittstelle bereitgestellt werden muss. Einige Anwendungen, z. B. Sensorwerte von mehreren verschiedenen Sensortypen, bei denen jeder Sensor geringfügig anders ist und geringfügig andere Daten generiert, reagieren gut auf diesen Ansatz.

Da Sie die Strukturen ohnehin Ihren Clients zur Verfügung stellen würden, opfern Sie ein wenig Typensicherheit (die zur Laufzeit noch überprüft werden kann) für eine viel einfachere API, auch wenn eine Umwandlung erforderlich ist.

Déjà vu

Wie ist ein undurchsichtiger Griff besser als eine benannte undurchsichtige Struktur?

Ich bin auf genau dasselbe Szenario gestoßen , nur mit einigen subtilen Unterschieden. Wir hatten in unserem SDK viele Dinge wie diese:

typedef void* SomeHandle;

Mein bloßer Vorschlag war, es unseren internen Typen anzupassen:

typedef struct SomeVertex* SomeHandle;

Für Dritte, die das SDK verwenden, sollte dies keinen Unterschied machen. Es ist ein undurchsichtiger Typ. Wen interessiert das? Dies hat keine Auswirkung auf die ABI * - oder Quellkompatibilität. Wenn Sie neue Versionen des SDK verwenden, muss das Plug-in trotzdem neu kompiliert werden.

_{* Beachten Sie, dass es, wie Gnasher betont, tatsächlich Fälle geben kann, in denen die Größe eines Zeigers auf struct und void * tatsächlich eine andere Größe hat. In diesem Fall würde sich dies auf ABI auswirken. So wie er bin ich in der Praxis noch nie darauf gestoßen. Aber von diesem Standpunkt aus könnte der zweite die Portabilität in einem obskuren Kontext verbessern, und das ist ein weiterer Grund, den zweiten zu bevorzugen, auch wenn dies für die meisten Menschen wahrscheinlich umstritten ist.}

Fehler von Drittanbietern

Darüber hinaus hatte ich noch mehr Anlass als Typensicherheit für die interne Entwicklung / Fehlersuche. Wir hatten bereits eine Reihe von Plugin-Entwicklern, die Fehler in ihrem Code hatten, weil zwei ähnliche Handles ( Panelund PanelNewdh beide) ein void*typedef für ihre Handles verwendeten und sie versehentlich die falschen Handles an die falschen Stellen weitergaben, weil sie nur die Handles verwendeten void*für alles. Es verursachte also Fehler auf der Seite der Benutzerdas SDK. Ihre Fehler kosteten das interne Entwicklungsteam auch enorm viel Zeit, da sie Fehlerberichte über Fehler in unserem SDK verschickten und wir das Plugin debuggen mussten und herausfanden, dass es tatsächlich durch einen Fehler im Plugin verursacht wurde, der die falschen Handles durchlief an die falschen Stellen (was ohne Warnung möglich ist, wenn jedes Handle ein Alias ​​für void*oder ist size_t). Wir haben also unnötig unsere Zeit verschwendet, um einen Debugging-Service für Dritte bereitzustellen, weil wir Fehler begangen haben, die durch unser Bestreben nach konzeptioneller Reinheit verursacht wurden, alle internen Informationen, auch die bloßen Namen unserer internen, zu verbergen structs.

Typedef behalten

Der Unterschied ist, dass ich vorschlug, dass wir uns an das typedefStandbild halten, um keine Clients zu schreiben, struct SomeVertexdie die Quellkompatibilität für zukünftige Plugin-Versionen beeinträchtigen würden . Ich persönlich mag die Idee, nicht structin C zu tippen , aus SDK-Sicht, aber das typedefkann helfen, da der springende Punkt die Undurchsichtigkeit ist. Daher würde ich vorschlagen, diesen Standard nur für die öffentlich zugängliche API zu lockern. Für Clients, die das SDK verwenden, sollte es unerheblich sein, ob ein Handle ein Zeiger auf eine Struktur, eine Ganzzahl usw. ist. Für sie ist nur wichtig, dass zwei verschiedene Handles nicht den gleichen Datentyp als Alias ​​haben, sodass dies nicht der Fall ist falsch in den falschen Griff an der falschen Stelle übergeben.

Typinformationen

Wo es am wichtigsten ist, Casting zu vermeiden, liegt bei Ihnen, den internen Entwicklern. Diese Art der Ästhetik, alle internen Namen aus dem SDK zu verbergen, ist eine konzeptionelle Ästhetik, die den erheblichen Preis für den Verlust aller Typinformationen mit sich bringt und erfordert, dass wir Casts unnötig in unsere Debugger einstreuen, um an kritische Informationen zu gelangen. Während ein C-Programmierer in C weitgehend daran gewöhnt sein sollte, ist es nur ein Problem, dies unnötig zu fordern.

Konzeptionelle Ideale

Im Allgemeinen sollten Sie auf diejenigen Entwicklertypen achten, die eine konzeptionelle Vorstellung von Reinheit weit über den praktischen, täglichen Bedarf stellen. Dadurch wird die Wartbarkeit Ihrer Codebasis verbessert, indem Sie nach einem utopischen Ideal suchen. Das gesamte Team vermeidet Sonnenschutzmittel in der Wüste aus Angst, dass es unnatürlich ist und einen Vitamin-D-Mangel verursachen könnte, während die Hälfte der Besatzung an Hautkrebs stirbt.

Benutzerendeinstellung

Würden sie selbst unter dem strengen Gesichtspunkt der Benutzer, die die API verwenden, eine fehlerhafte API oder eine API bevorzugen, die gut funktioniert, aber einen Namen preisgibt, den sie im Austausch kaum interessieren könnten? Denn das ist der praktische Kompromiss. Der unnötige Verlust von Typinformationen außerhalb eines generischen Kontexts erhöht das Risiko von Fehlern, und aufgrund einer umfangreichen Codebasis in einem teamweiten Umfeld über mehrere Jahre hinweg ist Murphys Gesetz in der Regel durchaus anwendbar. Wenn Sie das Risiko von Bugs überflüssig erhöhen, werden Sie wahrscheinlich noch einige Bugs bekommen. In einem großen Team dauert es nicht allzu lange, bis sich herausstellt, dass alle denkbaren menschlichen Fehler irgendwann von einem Potential in eine Realität verwandeln werden.

Vielleicht ist das eine Frage an die Benutzer. "Würden Sie ein fehlerhaftes SDK bevorzugen oder eines, das einige interne undurchsichtige Namen enthüllt, die Sie nie interessieren werden?" Und wenn diese Frage eine falsche Dichotomie aufwirft, würde ich sagen, dass mehr teamweite Erfahrung in einem sehr umfangreichen Umfeld erforderlich ist, um die Tatsache zu würdigen, dass ein höheres Risiko für Bugs letztendlich auf lange Sicht echte Bugs hervorruft. Es ist unerheblich, wie sehr der Entwickler sicher ist, Fehler zu vermeiden. In einem teamweiten Umfeld hilft es mehr, über die schwächsten Glieder nachzudenken, und zumindest über die einfachsten und schnellsten Möglichkeiten, um zu verhindern, dass sie auslösen.

Vorschlag

Daher würde ich hier einen Kompromiss vorschlagen, der Ihnen weiterhin die Möglichkeit gibt, alle Debugging-Vorteile beizubehalten:

typedef struct engine* enh;

... structwird uns das wirklich umbringen, selbst wenn wir die Schreibmaschine wegschreiben müssen ? Wahrscheinlich nicht, also empfehle ich Ihnen auch einen gewissen Pragmatismus, vor allem aber dem Entwickler, der es vorziehen würde, das Debuggen exponentiell zu erschweren, indem er size_thier verwendet und aus keinem guten Grund auf / von Ganzzahl umwandelt, außer um weitere Informationen auszublenden, die bereits 99 sind % für den Benutzer verborgen und kann unmöglich mehr Schaden anrichten als size_t.

Ich vermute, der wahre Grund ist Trägheit. Das haben sie schon immer getan und es funktioniert. Warum sollte man das ändern?

Der Hauptgrund, den ich sehe, ist, dass der Designer mit dem undurchsichtigen Griff überhaupt nichts dahinter setzen kann, nicht nur eine Struktur. Wenn die API mehrere undurchsichtige Typen zurückgibt und akzeptiert, sehen sie alle für den Aufrufer gleich aus, und es sind keine Kompilierungsprobleme oder Neukompilierungen erforderlich, wenn sich das Kleingedruckte ändert. Wenn sich en_NewFlidgetTwiddler (handle ** newTwiddler) ändert, um einen Zeiger anstelle eines Handles an den Twiddler zurückzugeben, ändert sich die API nicht und jeder neue Code verwendet unbeaufsichtigt einen Zeiger an der Stelle, an der er zuvor ein Handle verwendet hatte. Es besteht auch keine Gefahr, dass das Betriebssystem oder irgendetwas anderes den Zeiger stillschweigend "fixiert", wenn er Grenzen überschreitet.

Der Nachteil dabei ist natürlich, dass der Anrufer überhaupt etwas einspeisen kann. Sie haben eine 64-Bit-Sache? Schieben Sie es in den 64-Bit-Slot des API-Aufrufs und sehen Sie, was passiert.

en_TwiddleFlidget(engine, twiddler, flidget)
en_TwiddleFlidget(engine, flidget, twiddler)

Beide kompilieren, aber ich wette, nur einer von ihnen macht das, was Sie wollen.

Ich glaube, dass die Einstellung von einer langjährigen Philosophie herrührt, eine C-Bibliotheks-API vor Missbrauch durch Anfänger zu schützen.

Bestimmtes,

• Bibliotheksautoren wissen, dass es sich um einen Zeiger auf die Struktur handelt, und die Details der Struktur sind für den Bibliothekscode sichtbar.
• Alle erfahrenen Programmierer, die die Bibliothek verwenden, wissen auch, dass dies ein Hinweis auf einige undurchsichtige Strukturen ist.
  • Sie hatten genug schmerzhafte Erfahrung, um nicht mit den in diesen Strukturen gespeicherten Bytes herumzuspielen.
• Unerfahrene Programmierer wissen beides nicht.
  • Sie versuchen, memcpydie undurchsichtigen Daten zu ermitteln oder die Bytes oder Wörter in der Struktur zu erhöhen. Geh hacken.

Die langjährige traditionelle Gegenmaßnahme besteht darin,

• Maskieren Sie die Tatsache, dass ein undurchsichtiges Handle tatsächlich ein Zeiger auf eine undurchsichtige Struktur ist, die im selben Prozessspeicherbereich vorhanden ist.
  • Dies zu tun, indem behauptet wird, es sei ein ganzzahliger Wert mit der gleichen Anzahl von Bits wie a void*
  • Um besonders vorsichtig zu sein, maskieren Sie auch die Bits des Zeigers, z
    struct engine* peng = (struct engine*)((size_t)enh ^ enh_magic_number);

Dies ist nur zu sagen, dass es lange Traditionen hat; Ich hatte keine persönliche Meinung darüber, ob es richtig oder falsch ist.