non-blocking sockets - HTTP - Client-Seitig

dachschaden

temi schrieb:

Nur mal aus Neugier: Was ist daran so schlimm? IF..ELSE ist doch korrektes C++. Die beiden "return" könnten hier allerdings entfallen. Da die Funktion nach dem ELSE sofort endet würde auch das gehen

Kritischer Pfad und so. Deine Funktion hat eine Aufgabe; der Hauptpfad sollte nicht irgendwo abzweigen, sondern bis zum Ende durchgebracht werden.

Korrekt ist das natürlich. Aber auf mich macht das immer den Eindruck, dass die Person nicht über den Code nachgedacht hat. Ich war mal bei einer Firma, in der der Chefprogrammierer den kritischen Pfad in 5 oder 6 if-Scopes hatte (EDIT: über 1000 Zeilen). Was das für die Lesbarkeit bedeutete, muss ich dir hoffentlich nicht erklären.

temi schrieb:

Ich finde ja das sich "Wenn A oder B" leichter lesen lässt als "Wenn nicht A und nicht B".

Finde ich nicht. "Wenn nicht A und nicht B, dann raus. Ansonsten weitermachen." Wobei man sich das "ansonsten" direkt sparen kann, weil ja schon raus ist.

tntnet

dachschaden schrieb:

OK, dann denk' dir mal folgenden Sachverhalt:

- ich sage "X".
- du sagst, ich hätte "XYZ" gesagt.
- ich sage, du halluzinierst.

Das ist doch genau die Art und Weise, wie Du kommunizierst. Ich würde auf die "XYZ" aussage nicht mit einem Affront reagieren und meinem Kommunikationspartner angreifen, indem ich sage, dass er halluziniert. Eher würde ich ihn freundlich und respektvoll darauf aufmerksam machen, dass er mich offensichtlich falsch verstanden hat und versuchen, meine Aussage zu präzisieren. Da werfe ich ihm nichts vor, sondern halte es durchaus für möglich, dass ich mich unklar ausgedrückt habe. Die Reaktion "du halluzinierst" ist ein Vorwurf.

Suche mal nach ich und du Botschaften. Darüber ist viel geschrieben worden.

Und jetzt nochwas zum Thema:

dachschaden schrieb:

Wenn du einen Server aufmachst, machst du in der Regel einen listen -Aufruf. Dem übergibst du ein Backlog, also wie viele eingehende Verbindungen du so haben willst. Wenn ich dich DoSen wollte, würde ich also so viele Verbindungen aufmachen, wie ich wollte, damit die anderen keine Slots bekommen.

Der Backlog Parameter sagt nicht aus, wie viele Verbindungen der Socket aufnehmen kann. Er sagt lediglich aus, wie viele Verbindungsanfragen gesammelt werden, die noch nicht mit accept angenommen wurden. Schreibe ich einen Server, werde ich jede Verbindung unmittelbar mit accept annehmen und damit den Backlog abbauen.

Du machst auf mich den Eindruck, als wüsstest Du alles. Aber in dem Fall glaube ich, dass Du was falsch verstanden hast. Oder ich habe mal wieder Deine Erläuterung missinterpretiert oder eventuell sogar halluziniert?

Ach ja - und dieses if-else geraffel in meinem Code könnte ich tatsächlich klarer schreiben. Du hast Recht. Normalerweise achte ich sehr darauf, aber an der Stelle war ich offensichtlich ein wenig nachlässig. Ich ändere das gleich mal.

tntnet

Jetzt habe ich mir die Stelle im Code noch mal genauer angeschaut. Isoliert betrachtet sollte man das return weg lassen. Aber im größeren Kontext lasse ich es lieber drin. Es ist so klarer.

Im nächsten State sieht man es:

void HeaderParser::state_protocol0(char ch)
    {
        if (ch == ' ' || ch == '\t')
        {
            return;
        }
        else if (std::isalpha(ch))
        {
            token.reserve(32);
            token = ch;
            state = &HeaderParser::state_protocol;
            return;
        }
        else
        {
            log_warn("invalid character " << chartoprint(ch) << " in http protocol field");
            state = &HeaderParser::state_error;
            return;
        }
    }

Hier könnten die returns auch weg gelassen werden. Allerdings gleich im ersten if-Block steht ein einsames return. Logisch könnte ich es weg lassen, aber dann ist nicht auf dem ersten Blick klar, dass hier bei einem space oder tab nichts gemacht werden soll. Whitespace soll an der Stelle einfach übersprungen werden, führt also zu keinem anderen Zustand.

Daher halte ich es für sinnvoll, die returns konsistent im gesamten Parser genau so zu setzen. Leider habe ich mich aber selbst nicht daran gehalten, sondern in anderen Zuständen dann doch aufs return verzichtet. Konsistent ist das nicht. Ich denke, in dem Fall ist das aber entschuldbar, da jeder Zustand für sich sehr übersichtlich ist. Also ich verzeihe mir zumindest mal .

Das ist übrigens auch eine schöne Stelle, wo Daten vom Socket gelesen werden und gleich verworfen werden, da sie nicht relevant sind. Dafür brauche ich keinen Platz im Puffer. Auch andere Zeichen führen nur zu Zustandsübergängen und werden nicht zwischengespeichert.

Ein solcher zustandsbasierter Parser macht das parsen von komplexen Zusammenhängen relativ überschaubar.

Sicher könnte man bemängelt (und das tut Dachschaden ja auch), dass für jedes Zeichen ein Funktionsaufruf erfolgt. Ich habe andere Parser, wo das ganze in einem großen switch-case Ausdruck statt findet (z.B. src/uri.cpp). Das mit dem Aufruf haben wir mal ausprobiert. Bei Benchmarks haben wir keinen Performanceunterschied ausmachen können.

Wenn du überall return machst brauchst du aber auch kein else if mehr.

dachschaden

tntnet schrieb:

Das ist doch genau die Art und Weise, wie Du kommunizierst. Ich würde auf die "XYZ" aussage nicht mit einem Affront reagieren und meinem Kommunikationspartner angreifen, indem ich sage, dass er halluziniert.

Finde ich nicht. Wenn ich, wie du es formulierst, freundlich und respektvoll darauf aufmerksam mache, dann schone ich vielleicht deine Gefühle, aber du lernst dadurch nicht, es künftig zu vermeiden. Dadurch, dass ich direkt das Faktum nenne, wirst du in der Zukunft versuchen, diese Negativität zu vermeiden und die Texte deines Gegenüber korrekt zu lesen. Sonst ist der Anreiz einfach nicht groß genug.

Es kostet mich mehr Mühe und Zeit, fehlerhafte Aussagen durchzulesen und zu korrigieren, als dich, sie direkt korrekt niederzuschreiben.

Ob du das dadurch tust, indem du dein Umfeld änderst, oder indem du dich änderst, ist mir dann relativ latte. Ich bin nur halt gerne von Leuten umgeben, die schlauer sind als ich.

tntnet schrieb:

Der Backlog Parameter sagt nicht aus, wie viele Verbindungen der Socket aufnehmen kann. Er sagt lediglich aus, wie viele Verbindungsanfragen gesammelt werden, die noch nicht mit accept angenommen wurden. Schreibe ich einen Server, werde ich jede Verbindung unmittelbar mit accept annehmen und damit den Backlog abbauen.

man listen(2) schrieb:

The backlog argument defines the maximum length to which the queue of pending connections for sockfd may grow.

Hm. Ich habe immer "pending connections" als die Anzahl an Verbindungen gelesen, die der Socket offen haben lassen kann, nicht als die Anzahl an Verbindungen, die der Kernel noch vorhält. Aber der Absatz macht wohl deutlich, dass du recht hast:

man listen(2) schrieb:

The behavior of the backlog argument on TCP sockets changed with Linux 2.2. Now it specifies the queue length for completely established sockets waiting to be accepted, instead of the number of incomplete connection requests.

tntnet schrieb:

Es ist so klarer.

Ach?

void HeaderParser::state_protocol0(char ch)
{
    if (ch == ' ' || ch == '\t')
        return;

    if (!std::isalpha(ch))
    {
        log_warn("invalid character " << chartoprint(ch) << " in http protocol field");
        state = &HeaderParser::state_error;
        return;
    }

    token.reserve(32);
    token = ch;
    state = &HeaderParser::state_protocol;
    return;
}

Du findest also, dass das nicht klarer aussieht? Wenn ich mir die Funktion durchlese, sehe ich:

"Aha, wenn das Zeichen ein Whitespace ist, raus! Super! Aus den Augen, aus dem Sinn.
Wenn std::isalpha meint, es ist keines, loggen, Fehlerstatus setzen, raus! Wieder super!
Invaliditätsprüfung ist durch, jetzt geht's an Fleisch der Funktion: reserviere Speicher, packe das Zeichen schonmal rein, und markiere, dass es schonmal ganz gut aussieht."

Bei deinem Code denke ich:

"Aha, wenn das Zeichen ein Whitespace ist, raus! Super! Aus den Augen, aus dem Sinn ... nee, warte mal. Wenn nicht, dann prüfen, ob std::isalpha meint, es ist eines. Wenn es eines ist, dann ... ist das das Fleisch der Funktion? Reserviere Speicher, packe das Zeichen da rein, und markiere, dass es schonmal ganz gut aussieht? Sieht so aus. Ansonsten ... das heißt, wenn es kein Zeichen im Alphabet ist ... dann logge und setze Fehlerstatus."

Merke, wie ich beim ersten Mal direkt annehme, dass es sich hier um die Prüfung auf Korrektheit handelt (Invaliditätsprüfung). Das ist ein vertrautes Konzept, damit kann der Leser sofort arbeiten. Das habe ich kein einziges Mal bei dir im Code gedacht.

tntnet schrieb:

Das ist übrigens auch eine schöne Stelle, wo Daten vom Socket gelesen werden und gleich verworfen werden, da sie nicht relevant sind.

Das habe ich auch nicht angegriffen. Ich habe bereits gesagt, dass dein Code cache-lokaler ist als meiner, was durchaus ein Vorteil sein kann.

Auf der anderen Seite speichere ich mir ja auch mein Layout ab, und die Daten werden so oder so durch den Cache geschliffen (Schreiben von Kernel- in Userspace). Solange ich also beim nächsten recv dann wieder mit dem Prüfen anfange, macht mir das gar nichts, die neusten Daten sind bereits im Cache.

Was ist, wenn ich große Requests absetze? File-Upload bspw.? Oder bei Responses? Vielleicht habe ich die Antwort nur übersehen, aber gesehen habe ich noch keine.

EDIT: Warum immer dieser Code hier?

token.reserve(32);
token = ch;
state = &HeaderParser::state_protocol;

Nach dem dritten mal würde ich mir denken: "Komm, machen wir mal 'ne Funktion draus:

/*Bin gerade zu faul, den Typen von state zu suchen ...*/
inline void add_token
(
    state_type new_state,
    char ch,
    size_t reserve_size
)
{
    token.reserve(reserve_size);
    token = ch;
    state = new_state;
}

, und fertig ist die Laube."

Wenn ich ganz lustig wäre, dann würde ich noch eine inline-Funktion schreiben:

inline void add_token_with_default_size
(
    state_type new_state,
    char ch
)
{
    add_token(new_state,ch,32);
}

Ok ich hab mir den Code jetzt auch mal kurz angesehen.

Tausend Zeilen Code für so einen popeligen Header? Dann ein Funktionsaufruf per Zeichen noch dazu? Das find ich recht erschlagend.

Warum benutzt du keine Streams. Den ganzen Header-Parser hättest du auch mit 3 Methoden und 50 Zeilen Code machen können. Fehlerprüfung inklusive.

tntnet

Ob man jetzt if else oder return oder sonst was schreibt ist denke ich mal relativ egal. So wie es ist, finde ich es schon einigermaßen lesbar. Ich glaube, dass das nicht wirklich relevant ist. Ich habe es mir ja schon verziehen. Vielleicht tut ihr es ja auch.

@Blockade: Und so ein paar poplige Header sind das nicht. Schau Dir rfc 2616 an. Wenn Du jede Nuance ganz allgemeingültig unterstützen möchstest, ist das nicht mit ein paar string.find-Funktionen getan. Schau Dir einfach mal die continuation header an. Da wird es immer schwerer. Einen stateful parser finde ich persönlich einfacher zu warten und robuster. Es hat sich bewährt.

Ein einfacher Parser ist schnell geschrieben aber ein robuster vollständig standardkonformer macht da schon ein wenig mehr Arbeit.

Und ja, es werden Streams benutzt. Ich weiß nicht, wo Du an der Stelle Streams sehen willst.

Es ist auch Code, den man als normaler Benutzer nicht beachten muss. Dafür sind ja Libraries da.

tntnet

dachschaden schrieb:

tntnet schrieb:

Das ist doch genau die Art und Weise, wie Du kommunizierst. Ich würde auf die "XYZ" aussage nicht mit einem Affront reagieren und meinem Kommunikationspartner angreifen, indem ich sage, dass er halluziniert.

Finde ich nicht. Wenn ich, wie du es formulierst, freundlich und respektvoll darauf aufmerksam mache, dann schone ich vielleicht deine Gefühle, aber du lernst dadurch nicht, es künftig zu vermeiden. Dadurch, dass ich direkt das Faktum nenne, wirst du in der Zukunft versuchen, diese Negativität zu vermeiden und die Texte deines Gegenüber korrekt zu lesen. Sonst ist der Anreiz einfach nicht groß genug.

Ach, das ist wieder das. Du hältst Dich für unfehlbar. Wenn einer was falsch versteht, was Du gesagt hast, muss der Kommunkationspartner halluzinieren. Dass Du etwas unverständlich formulierst ist völlig undenkbar und Du musst unbedingt und in aller Deutlichkeit dieses Faktum betonen, dass der Partner Deine Aussage nicht korrekt gelesen hat. Und Du musst auch den Kommunikationpartner erziehen, dass er doch Deine ach so wertvollen und wohlformulierten Worte korrekt zu lesen hat. Das nenn ich mal überheblich und "Ego-Trip".

dachschaden

tntnet schrieb:

Ach, das ist wieder das. Du hältst Dich für unfehlbar. Wenn einer was falsch versteht, was Du gesagt hast, muss der Kommunkationspartner halluzinieren. Dass Du etwas unverständlich formulierst ist völlig undenkbar und Du musst unbedingt und in aller Deutlichkeit dieses Faktum betonen, dass der Partner Deine Aussage nicht korrekt gelesen hat.

OK, jetzt reicht's.

Gerade erst in dem letzten Post habe ich zugegeben, dass ich die verschissene Manpage nicht richtig gelesen habe. Das kann man als Halluzination interpretieren. Ich hätte es sogar verstanden. Aber vor allem: ich hatte erwartet, dass du das auch raffst.

Tust du nicht. Confirmation Bias. Alles, was dir gefällt, sieht du, das andere siehst du nicht. Du halluzinierst gerade wieder und siehst es nicht einmal. Kannst du gerne als Beleidigung auffassen.

Viel Spaß noch auf deiner Halluzination.

Ja.

Ihr habt mir den halben Thread zugekackt.

Mechanics

Blockade schrieb:

Ihr habt mir den halben Thread zugekackt.

YMMD

tntnet

Blockade schrieb:

Ja.

Ihr habt mir den halben Thread zugekackt.

War die andere Hälfte denn wenigstens hilfreich? Wenn ja, dann bin ich zufrieden. Wenn Nein, dann können wir gerne noch über die eigentliche Frage diskutieren.

Ich würde mir aber eine gepflegte Ausdrucksweise wünschen .

Dachschaden hat ja gesagt, dass es ihm reicht. Ich fürchte, er hält sich nicht dran, aber wir können es ja mal versuchen.

SeppJ

hä? schrieb:

SeppJ warum greifst du hier komischerweise nicht ein?

Weil's ein obskurer Thread im Linuxforum ist.