Fragen über Fragen ;)

cooky451

Guten,

ich versuche mich gerade daran mal eine "lib" zu schreiben - habe so etwas aber noch nie gemacht und hätte da noch ein paar, naja, organisatorische Fragen.

1. Die Großzahl der Funktionen habe ich als "static" deklariert. Soweit ich das verstanden habe, sollte man das auch so machen und nur die "Schnittstellen-" Funktionen als auto-extern belassen. Damit soll vor falschem Zugriff auf kritische Funktionen geschützt werden. Nur, wie organisiert man jetzt seine nicht-schnittstellen-Funktionen? Denn alle statischen Funktionen die keine eigene offene externe Funktion haben müssen so doch in einer Datei sein? Oder übersehe ich hier etwas ganz fundamentales? Beispiel:

- aufgabenbereich1.c -> nur statische Funktionen
- main.c -> Schnittstelle (benötigt Funktionen aus aufgabenbereich1.c - kann diese aber nicht sehen)

2. Errorhandling
Momentan habe ich eine Funktion "void foobar_set_notification_options". Dieser können FILE Pointer für die Ausgabe von Fehlern, Informationen, usw. übergeben werden. Möchte man bestimmte Nachrichten gar nicht haben - übergibt man einfach einen NULL Pointer. Zudem kann man den Parameter "use_event_queue" auf true oder false setzen. Steht dieser auf true, werden alle "events" auf eine queue "gepusht", die man dann später nach belieben wieder "runter-popen" kann
Die events bestehen zwar sowohl aus einer für Menschen lesbaren Nachricht (char*) allerdings auch aus drei ints welche für die Art der Nachricht, Fehlernummer und eventuell noch Art des Fehlers stehen. Ich erhoffe mir so eine schnelle und relativ simple Fehlerbehandlung.
Frage: Ist das beschriebende für eine lib so in etwa sinnvoll? Wenn nicht, wie regelt man sein "errorhandling" vorteilhafter?

3. Implementierung der queue
Ich habe zu erst einfach mal "frei Hand" eine queue geschrieben. Diese funktioniert auch einwandfrei und trotzdem wollte ich mal wissen wie andere sowas implementiert haben. Also einfach mal bei google nach sowas wie "queue c" gesucht und mir die Treffer angesehen. Nun, danach war ich irgendwie etwas verwirrt. Viele haben mit doppelt verketteten Listen gearbeitet, oder auch gerne mal mit extra Pointern auf den Anfang(?) der queue. Dieser ist doch aber IMMER queue[0] oder wie jetzt?
Das Ganze hat mich etwas verunsichert (habe so etwas noch nie gemacht) und deswegen poste ich jetzt einfach mal meine Version - und ihr sagt mir (hoffentlich ;)) ob und wenn ja, warum diese schlecht ist.

So ich hoffe der Text war nicht zu erdrückend und die Fragen einigermaßen verständlich - vielen Dank für eure Hilfe schonmal im voraus

static int event_queue(ytdl_event *event, int push)
{
  static int counter = 0;
  static ytdl_event *queue = 0;
  if (push)
  {
    if (!(queue = realloc(queue, ++counter * sizeof(ytdl_event))))
      return 1;
    queue[counter - 1] = *event;
  }
  else
  {
    if (!counter)
      return 1;
    *event = queue[0];
    if (!(--counter))
      return (int)(queue = realloc(queue, counter * sizeof(ytdl_event)));
    memmove(&queue[0], &queue[1], counter * sizeof(ytdl_event));
    return (int)!(queue = realloc(queue, counter * sizeof(ytdl_event)));
  }
  return 0;
}

Der Gedanke ist prinzipiell nicht dämlich, aber extern/static ist in der Regel nicht der richtige Mechanismus dafür. Häufig ist es ja doch so, dass man bestimmte Hilfsfunktionen, die mit dem API nichts zu tun haben, in mehreren Übersetzungseinheiten benutzen will - da hilft static einem nicht weiter. Außerdem reicht extern nicht in allen Umgebungen für den Export aus einer Bibliothek aus (ich kucke da in deine Richtung, MSVC!).

Glücklicherweise ist das Problem Compilerentwicklern bekannt, und es gibt Mechanismen dafür. Ich arbeite ausschließlich mit gcc und msvc, also musst du bei anderen Compilern in die Dokumentation kucken, aber bei den beiden geht das wie folgt:

gcc: Übersetze die Bibliothek mit -fvisibility=hidden und deklariere API-Funktionen mit __attribute__((visibility("default"))), beispielsweise

void __attribute__((visibility("default"))) api_function(int, double, struct foo*);

msvc: Hier ist es insofern etwas komplizierter, als dass beim Bauen der DLL und beim Einbinden selbiger unterschiedliche Mechanismen verwendet werden müssen. Für den Export:

void __declspec(dllexport) api_function(int, double, struct foo*);

für den Import

void __declspec(dllimport) api_function(int, double, struct foo*);

Man hilft sich dabei üblicherweise mit Makros, etwa

#ifdef BUILDING_DLL
#  define DLLPUBLIC __declspec(dllexport)
#else
#  define DLLPUBLIC __declspec(dllimport)
#endif

void DLLPUBLIC api_function(int, double, struct foo*);

Natürlich sollte man die Makronamen möglichst eindeutig wählen, etwa durch ein eindeutiges, deiner Bibliothek zugeordnetes Präfix (*MYLIB*_BUILDING_DLL, *MYLIB*_DLLPUBLIC etc.). Um das ganze portabel zu machen empfiehlt sich etwas dieser Art:

#ifdef _MSC_VER /* MSVC */
#  ifdef BUILDING_DLL
#    define DLLPUBLIC __declspec(dllexport)
#  else
#    define DLLPUBLIC __declspec(dllimport)
#  endif
#elif defined(__GNUC__)
#  define DLLPUBLIC __attribute((visibility("default")))
/* ggf. Code für andere Compiler hier */
#else
#  define DLLPUBLIC
#endif

Alternativ gibt es bei MSVC die Möglichkeit, beim Kompilieren eine .def-Datei anzugeben, die die zu exportierenden Funktionsnamen enthält, siehe dazu hier. Allerdings bedeutet das natürlich doppelte Buchführung, gerade wenn das ganze nicht nur unter Windows laufen soll.

Das klingt für mich eher nach Logging als nach Fehlerbehandlung. Logging kann man auf diese Weise wohl abhandeln, Fehlerbehandlung erfordert aber, dass Fehlercodes in einer Weise zurückgegeben werden kann, in der das benutzende Programm die Fehler zur Laufzeit sinnvoll behandeln kann. So was ist in C mangels Exceptions eine vergleichsweise haarige Angelegenheit; ein gängiger Mechanismus ist es, den Rückgabewert der Funktion dafür zu benutzen. Einfaches Beispiel:

int quadratwurzel(double *result, double x) {
  if(x < 0) {
    return EDOM;
  }

  *result = sqrt(x);
  return 0;
}

oder auch

int quadratwurzel(double *result, double x) {
  if(x < 0) {
    errno = EDOM;
    return -1;
  }

  *result = sqrt(x);
  return 0;
}

Das ist vielleicht nicht das beste Beispiel (sqrt setzt errno im Fehlerfall bereits auf EDOM), aber die Idee sollte klar werden. Komfortabel ist was anderes, das geb ich gerne zu, aber für Komfort ist C leider nicht die richtige Sprache.

Ich würde den statischen Kram sein lassen; damit handelst du dir nur Ärger ein. Spätestens, wenn du Multithreading unterstützt und/oder mehrere Queues brauchst, wird so was ekelhaft.

Spricht etwas dagegen, das ganze objektorientiert aufzuziehen, etwa

typedef struct  {
  ytdl_event *events;
  size_t size;
  size_t capacity;
} ytdl_event_queue;

void ytdl_queue_init(ytdl_event_queue *queue);
int  ytdl_queue_push(ytdl_event_queue *queue, ytdl_event *event);
int  ytdl_queue_pop (ytdl_event_queue *queue, ytdl_event *event_dest);

?

Abhängig davon, wie viele Events du erwartest, in der Queue zu halten, könnte sich allerdings aus Performance-Gründen eine verkettete Liste oder ein Ringpuffer besser eignen. Jedenfalls würde ich an deiner Stelle die Speicherverwaltung überdenken und mindestens blockweise Speicher anfordern, anstatt für jedes Event die ganze Queue zu verlegen.

hustbaer

@ 2. Errorhandling:

Was den "logging" Teil angeht: ich ziehe es vor, wenn eine Library eine "set_error_handler" Funktion hat, der ich einen Funktionszeiger + einen void* userdata übergeben kann. (Der Userdata-Zeiger wird dann der Funktion auf die der Funktionszeiger zeigt beim Aufruf wieder mitgegeben.)

Was Error-Queues angeht: sowas mag ich gar nicht. Ich frag mich dann immer was ich damit machen soll, wo ich die vernünftig durchackern kann, ob ich sie ausleeren muss oder nicht (=ob sie den ganzen Speicher aufessen wenn man sie nie leer macht) etc.
Wenn ich eine "handler Funktion" übergeben kann brauche ich auch keine Error-Queues, kann ich mir ja selbst basteln wenns wirklich nötig ist.

Was Fehlerbehandlung selbst angeht: das Üblichste ist wirklich Returnwerte zu verwenden. Allerdings kannst du dann a) den eigentlichen Returnwert nicht mehr als Returnwert zurückgeben und b) nur einen int/long, und keine Zusatz-Infos.

Hier gibt es zwei Möglichkeiten die - je nachdem was man braucht - besser sein können:

Einen "last error" Speicher, d.h. eine GetLastError() Funktion. Dummerweise ist das problematisch sobald man mit mehreren Threads arbeitet. Wenn man sich die Arbeit antun möchte das ganze Threas-safe zu machen (d.h. jeder Thread hat seinen eigenen "last error" Speicher), dann ist es OK. Allerdings steht der Aufwand IMO nicht dafür. (Es sei denn der verwendete Compiler macht Thread-Local-Storage wirklich einfach, dann könnte man drüber nachdenken.)
Man verpasst jeder Funktion einen zusätzlichen Parameter, der auf eine ErrorInfo struct zeigt. Jede Funktion macht dann sinngemäss folgendes:

int Foo(int a, int b, int c, ErrorInfo* err)
{
    // Always check err->ErrorCode *BEFORE* actually doing anything
    if (err && err->ErrorCode)
        return 0; // safe-default

    // ...

    if (...)
    {
        // Fehler
        if (err)
        {
            err->ErrorCode = ...;
            err->SomeOtherValue = ...;
            strcpy(&(err->Description), "Blah");
        }
        return 0; // safe-default
    }

    // ...

    return 42;
}

(Wenn man den ErrorInfo Zeiger verpflichtend macht, kann man sich natürlich einiges an Code in der Implementierung sparen. Macht die Verwendung dann aber eine Spur lästiger, da man jeden Aufrufer zwingt immer eine ErrorInfo struct zu initialisieren und mitzugeben.)

Das ist einiges mehr an Aufwand in der Library selbst, dafür kann man es schön verwenden. Man muss dadurch nämlich oft nicht mehr nach jedem Funktionsaufruf auf Fehler prüfen, sondern kann oft etliche Library-Funktionen hintereinander aufrufen, und den Fehler-Wert erst ganz zum Schluss prüfen. Beispiel:

FOO* foo;
ErrorInfo err = {0};

// Viele Library-Aufrufe ...
foo = foo_open(&err);

foo_write_header(foo, &err);
foo_write_data(foo, "bar", &err);
foo_write_data(foo, "baz", &err);
foo_flush(foo, &err);

foo_close(foo, &err);

// ... und nur ein "if", und trotzdem alle Fehler gecheckt :^)
if (err.ErrorCode)
{
    printf("Scheissendreck, irgendwas hat beim Foo-Schreiben nicht funktioniert!
\n");
    return Waaaaaaaah_Fehler;
}

// ...

Ist ein wenig wie "Exceptions für Arme"

hustbaer

ps: man kann natürlich auch einen Zeiger auf einen Zeiger auf ein ErrorInfo als Parameter übergeben, und die Funktion legt die ErrorInfo struct nötigenfalls selbst an. Erfordert dann allerdings vom User dass er sich ums Freigeben kümmert. Was viele wohl einfach ganz fest ignorieren werden
Und es erfordert dass man im Fehlerfall dynamisch Speicher anfordert.
Alles in allem also vermutlich ein Schritt zurück.

also ich weiß nicht, ob ihrs mit der fehler behandlung nicht übertreibt... normal kann doch ein programm nicht so viele fehler haben, dass ma da gleich ein derart ausgefuchstes errorhandling braucht. und besonders in libs, denn diese sollten nach möglichkeit keine eingabe/ausgabe/hardwarezugriffe durchführen. und wenn man sowas braucht, muß man sich sowieso ans errorhandling des os halten!

hustbaer

Alter Schwede..

Soviel Unsinn auf einmal, das muss man erstmal schaffen.
Viel mehr will ich jetzt gar nicht dazu sagen, erstens keine Zeit und zweitens führt es ja doch zu nix.

hustbaer schrieb:

Alter Schwede..

Soviel Unsinn auf einmal, das muss man erstmal schaffen.
Viel mehr will ich jetzt gar nicht dazu sagen, erstens keine Zeit und zweitens führt es ja doch zu nix.

ja, es ist eben eine frage der umsetzung und das was du hier an bsp. code an start gebracht hast ist einfach grottig!

cooky451

@Problem 1:
Gibt es keine standard C - also Compilerunabhängige Methode? Falls nicht, werde ich wohl einfach gcc nehmen und diesen Compiler vorraussetzen.

@Problem 2:
Wenn ich das mit dem set_error_handler richtig verstanden habe, würde das dazu führen das die lib fremden Code ausführt.. finde ich nicht so schön. Auch bei GetLastError() sehe ich keinen wirklichen Vorteil gegenüber einer Queue. Das Argument mit "muss ich die events wieder runter ziehen" kann ich nicht so ganz nachvollziehen, da man die Queue ja mit set_notification_options vollständig deaktivieren kann.
Zudem hatte ich mir eh überlegt die Queue dann in einem Ringpuffer zu implementieren. Hier würde dann zwar dauerhaft Speicher belegt, aber mal ehrlich - ein event würde 16 byte groß sein. Selbst wenn ich den Ringpuffer 4096 Stellen groß anlegen würde (was schon viel zu groß wäre für die paar events) wären dass immer noch gerade mal 65536 belegte bytes, also 64 KB Speicherverbrauch. Das ist auf heutigen Rechnern wohl so ziemlich ****** egal

Wie ich mir das also vorgestellt hatte:
Innerhalb der lib nutze ich eh die Funktion post_error bzw. post_info usw.
zB.:

if (!(p = malloc(sizeof(blah))))
  return post_error(error_code, sec_error_code, "msg", ...);

Die Funktion post_error reagiert nun je nach gewünschten Einstellungen. Der Fehler kann in die Queue gepusht oder aber auch in einen übergebenden FILE Pointer geschrieben werden(wobei hier nur die msg angezeigt wird). Auch beides ist möglich.

Textbasierte Programme hätte die Möglichkeit stderr und stdout zu übergeben, und die Queue einfach zu deaktivieren. Dann müssten sie sich um Rückgabewerte auch nicht mehr kümmern, Fehler werden ja bereits Menschenlesbar angezeigt. (Ein Programm an sich hätte hier eh keine Möglichkeit automatisch sinnvoll auf einen Fehler der lib zu reagieren).

GUI Programme könnten so ganz leicht mit

if ((event = ytdl_dequeue_event()) != no_event)
  ..

auf alle aufkommenden Informationen / Fehler reagieren.

@hustbaer
Deswegen habe ich hier noch nicht so ganz verstanden was dir daran nicht gefällt, deine Argumente standen eher gegen schlecht Dokumentierte libs.. (Die sind schrecklich, das stimmt :D)

@Seldon:
Mit return Werten kann ich aber keine Status Informationen verarbeiten
Edit:
Na gut hattest Du ja schon angemerkt. Aber auch bei Threads wirds mit return Werten nervig - zudem muss man dann einen return Wert auch immer einem Fehler zuordnen und anzeigen usw. Und wie siehts mit verschachtelten Funktionen aus? Das macht das Ganze dann wieder unnötig kompliziert..

@__--:
Genau, eine lib sollte nach Möglichkeit niemals Fehler machen, dann braucht man auch keine Fehlerbehandlung!

@Problem 3:
Habe mal so eine Art Ringpuffer-ding geschrieben soweit ich das was bei Wikipedia stand richtig verstanden habe. Ob das jetzt aber auch eleganter ist.. naja, ich poste es einfach mal: (Hoffe da sind keine Fehler drin ;))

#define QUEUE_BUFFER_SIZE 0x100
static ytdl_event queue[QUEUE_BUFFER_SIZE];
static ytdl_event *read_pos = &queue[0];
static ytdl_event *write_pos = &queue[0];
static unsigned int entries = 0;
ytdl_event no_event = {0};

void ytdl_enq_event(ytdl_event *event)
{
  if (entries < QUEUE_BUFFER_SIZE)
    ++entries;
  if (write_pos >= &queue[QUEUE_BUFFER_SIZE])
    write_pos = &queue[0];
  *write_pos++ = *event;
}

ytdl_event ytdl_deq_event()
{
  if (entries <= 0)
    return no_event;
  --entries;
  if (read_pos >= &queue[QUEUE_BUFFER_SIZE])
    read_pos = &queue[0];
  return *read_pos++;
}

wart lieber noch paar jahre, bis du anfängst libs zu proggen, am ende verwendet die noch jemand...

cooky451

__-- schrieb:

wart lieber noch paar jahre, bis du anfängst libs zu proggen, am ende verwendet die noch jemand...

Du bist wirklich der Hilfreichste, besten Dank!

hustbaer

__-- schrieb:

hustbaer schrieb:

Alter Schwede..

Soviel Unsinn auf einmal, das muss man erstmal schaffen.
Viel mehr will ich jetzt gar nicht dazu sagen, erstens keine Zeit und zweitens führt es ja doch zu nix.

ja, es ist eben eine frage der umsetzung und das was du hier an bsp. code an start gebracht hast ist einfach grottig!

Was denn zum Beispiel, grosser Meister Strichi-Strichi?

Gigaz

Das Problem hier hat sich erledigt.