Hallo Zusammen,
ich sehe den Wald vor lauter Bäumen nicht mehr. Wenn ich untenstehender
Code mit Debug kompiliere klappt alles, im Release Modus bekomme ich ein
SIGSEGV.
Hat jemand einen Tipp was ich übersehe?
René H. schrieb:> sprintf(dateStr, "%02d.%02d.%04d", my_tm->tm_mday,> my_tm->tm_mon+1, my_tm->tm_year+1900);> sprintf(timeStr, "%02d:%02d:%02d", my_tm->tm_hour,> my_tm->tm_min, my_tm->tm_sec);
Wenn du da schon rumprinten musst, nimm wenigstens snprintf oder sowas.
Wenn da Schrott in der struct steht, schreibst das so locker über die
bounds. Sträflicher code.
Ansonsten empfehle ich valgrind oder einen sanitizer.
Heiko L. schrieb:> René H. schrieb:> sprintf(dateStr, "%02d.%02d.%04d", my_tm->tm_mday, my_tm->tm_mon+1,> my_tm->tm_year+1900);> sprintf(timeStr, "%02d:%02d:%02d", my_tm->tm_hour,> my_tm->tm_min, my_tm->tm_sec);>> Wenn du da schon rumprinten musst, nimm wenigstens snprintf oder sowas.> Wenn da Schrott in der struct steht, schreibst das so locker über die> bounds. Sträflicher code.>> Ansonsten empfehle ich valgrind oder einen sanitizer.
Egal welcher Schrott da steht (was man bei gmtime() getrost ignorieren
kann, sind die Bounds terminiert). Nicht das Problem.
René H. schrieb:> Egal welcher Schrott da steht (was man bei gmtime() getrost ignorieren> kann, sind die Bounds terminiert). Nicht das Problem.
Ja, 32 bit? -2147483648 = 11 Zeichen * 3 + 3 = 36.
Mit sprintf sagt man halt: "Schreib hin wo du willst."
Da kann man sicher auf den Exploit warten, der über einen Hack der
Date-API root-Zugriff erzeugt.
Heiko, wenn Du es schaffst von aussen das Firmennetzwerk zu hacken und
den Exploit zu setzen, bist du ein gemachter Mann für den Rest Deines
Lebens.
Gelingt es aber nicht (was sehr wahrscheinlich ist), landest Du ob dem
ersten Versuch im Knast. ;-)
So, have fun.
René H. schrieb:> Heiko, wenn Du es schaffst von aussen das Firmennetzwerk zu hacken> und> den Exploit zu setzen, bist du ein gemachter Mann für den Rest Deines> Lebens.>> Gelingt es aber nicht (was sehr wahrscheinlichist), landest Du ob dem> ersten Versuch im Knast. ;-)>> So, have fun.
Ich meine nur. Solche ungeprüften writes sind DER Einstiegspunkt
schlechthin. Schon als ich damals C gelernt habe, stand das in jedem
Tutorial. "Nie, nie, niemals, nie einfach sprintf."
René H. schrieb:> Wilhelm M. schrieb:>> Ja, wo ist denn das return o;>>>> Außerdem: assert(my_tm != nullptr);>> Aaaargh!!! Thx.>> Grüsse,> René
Wohl mal wieder keine Warnungen eingeschaltet?
Heiko L. schrieb:> René H. schrieb:>> Heiko, wenn Du es schaffst von aussen das Firmennetzwerk zu hacken>> und>> den Exploit zu setzen, bist du ein gemachter Mann für den Rest Deines>> Lebens.>>>> Gelingt es aber nicht (was sehr wahrscheinlichist), landest Du ob dem>> ersten Versuch im Knast. ;-)>>>> So, have fun.>> Ich meine nur. Solche ungeprüften writes sind DER Einstiegspunkt> schlechthin. Schon als ich damals C gelernt habe, stand das in jedem> Tutorial. "Nie, nie, niemals, nie einfach sprintf."
Ja, wenn sie unformatiert sind, in dem Fall sind sie formatiert.
Grüsse,
René
René H. schrieb:> Ja, wenn sie unformatiert sind, in dem Fall sind sie formatiert.
Du gibst aber nur die Minimalfeldgröße an mit Füllzeichen. Es kann auch
größer werden. Da gibt es den Modulo-trick.
> Ja, wenn sie unformatiert sind, in dem Fall sind sie formatiert.>> Grüsse,> René
Also wenn du dir sicher sein kannst, dass der Code nie auf 32bit läuft,
dann kann man doch auch sicher sein, dass ein %s-Eingabe-String nie
länger als X Zeichen ist, oder nicht?
Nichts für ungut.
Heiko L. schrieb:>> Ja, wenn sie unformatiert sind, in dem Fall sind sie formatiert.>>>> Grüsse,>> René>> Also wenn du dir sicher sein kannst, dass der Code nie auf 32bit läuft,> dann kann man doch auch sicher sein, dass ein %s-Eingabe-String nie> länger als X Zeichen ist, oder nicht?>> Nichts für ungut.
Wo siehst Du %s? Der Code läuft auf einem 64Bit / 48 CPU System.
Wilhelm M. schrieb:> René H. schrieb:>> Ja, wenn sie unformatiert sind, in dem Fall sind sie formatiert.>> Du gibst aber nur die Minimalfeldgröße an mit Füllzeichen. Es kann auch> größer werden. Da gibt es den Modulo-trick.
Nope, kann es nicht. Sonst zeig mir wie .....
René H. schrieb:> Heiko L. schrieb:>>> Ja, wenn sie unformatiert sind, in dem Fall sind sie formatiert.>>>>>> Grüsse,>>> René>>>> Also wenn du dir sicher sein kannst, dass der Code nie auf 32bit läuft,>> dann kann man doch auch sicher sein, dass ein %s-Eingabe-String nie>> länger als X Zeichen ist, oder nicht?>>>> Nichts für ungut.>> Wo siehst Du %s? Der Code läuft auf einem 64Bit / 48 CPU System.
Nein, das war eine Analogie: Ein sprintf(stack, "%s", something) ist
offensichtlicher irgendwie gefährlich. Bei %Xd hat man immerhin noch
eine gewisse Größenvorstellung. Doch wie Wilhelm schon sagt
1
(number) Minimum number of characters to be printed. ...
"Es können durchaus mehr werden."
Da ist man entweder so hart, zu sagen "Auf den Architekturen, wo das
laufen wird ist der Zahlentyp maximal so und so lang" (was dann
irgendwann kaputt geht, wenn der Code in weniger heimischer Umgebung
herumstreunt) oder man sichert das halt von vornherein ab.
Im vorliegenden Fall: Warum nicht direkt mit dem stream arbeiten?
Wilhelm M. schrieb:> René H. schrieb:> Nope, kann es nicht. Sonst zeig mir wie .....>> Sicher:printf("%02d\n", 12345678);
Yep, sicher!
Getestet.
Grüsse,
René
Heiko L. schrieb:> René H. schrieb:>> Heiko L. schrieb:>>>> Ja, wenn sie unformatiert sind, in dem Fall sind sie formatiert.>>>>>>>> Grüsse,>>>> René>>>>>> Also wenn du dir sicher sein kannst, dass der Code nie auf 32bit läuft,>>> dann kann man doch auch sicher sein, dass ein %s-Eingabe-String nie>>> länger als X Zeichen ist, oder nicht?>>>>>> Nichts für ungut.>>>> Wo siehst Du %s? Der Code läuft auf einem 64Bit / 48 CPU System.>> Nein, das war eine Analogie: Ein sprintf(stack, "%s", something) ist> offensichtlicher irgendwie gefährlich. Bei %Xd hat man immerhin noch> eine gewisse Größenvorstellung. Doch wie Wilhelm schon sagt>
1
> (number) Minimum number of characters to be printed. ...
2
>
> "Es können durchaus mehr werden."> Da ist man entweder so hart, zu sagen "Auf den Architekturen, wo das> laufen wird ist der Zahlentyp maximal so und so lang" (was dann> irgendwann kaputt geht, wenn der Code in weniger heimischer Umgebung> herumstreunt) oder man sichert das halt von vornherein ab.>> Im vorliegenden Fall: Warum nicht direkt mit dem stream arbeiten?
Ich bau es nach sstream um, war bloss zu faul. Danke für die Hinweise!
Grüsse,
René
Wilhelm M. schrieb:> René H. schrieb:>> Nope, kann es nicht. Sonst zeig mir wie .....>> Sicher:> printf("%02d\n", 12345678);
Mist!!! Ihr habt Recht :-(
Wilhelm M. schrieb:> René H. schrieb:>> sprintf(str, "%02d", 123456);>> Da gibt es den Modulo-Trick:> sprintf(str, "%02d", 123456 % 100);
Das ist gut, ich werde es aber trotzdem auf sstream umbauen....
Aber (so hoffe ich), dass das fehlende Return zum SIGSEGV geführt hat
(ich kann es mir nicht vorstellen, möglich ist es dennoch). Ich sehe
sonst keine Möglichkeit für Speicherüberschreibungen (muss dem sprintf,
wie ich ja jetzt weiss).
Grüsse,
René
René H. schrieb:> Aber (so hoffe ich), dass das fehlende Return zum SIGSEGV geführt hat> (ich kann es mir nicht vorstellen, möglich ist es dennoch).
auf jeden Fall
René H. schrieb:> Aber (so hoffe ich), dass das fehlende Return zum SIGSEGV geführt hat> (ich kann es mir nicht vorstellen, möglich ist es dennoch).
Also ich kann bestätigen, dass fehlende returns mitunter SEGVs erzeugen:
Das ist mir nämlich auch schon passiert. (Dabei hatte ich aber sogar
noch warnings ignoriert shame.)
Wird mal Zeit, -Werror zu benutzen.
Wilhelm M. schrieb:> René H. schrieb:>> Wilhelm M. schrieb:>>> Ja, wo ist denn das return o;>>>>>> Außerdem: assert(my_tm != nullptr);>>>> Aaaargh!!! Thx.>>>> Grüsse,>> René>> Wohl mal wieder keine Warnungen eingeschaltet?
Nope! Die ärgern doch bloss :-)
Es wurde schon genannt, dass gmtime ein NULL zurückliefern kann. Ein
weiteres Problem angesichts des "ThreadControl" ist, dass gmtime nicht
Thread-safe ist - dafür gibt es gmtime_r.
foobar schrieb:> Es wurde schon genannt, dass gmtime ein NULL zurückliefern kann. Ein> weiteres Problem angesichts des "ThreadControl" ist, dass gmtime nicht> Thread-safe ist - dafür gibt es gmtime_r.
Hmm..., das ist im Main Thread und sollte somit kein Problem darstellen.
Aber das assert hab ich eingebaut.
René H. schrieb:> foobar schrieb:>> Es wurde schon genannt, dass gmtime ein NULL zurückliefern kann. Ein>> weiteres Problem angesichts des "ThreadControl" ist, dass gmtime nicht>> Thread-safe ist - dafür gibt es gmtime_r.>> Hmm..., das ist im Main Thread und sollte somit kein Problem darstellen.> Aber das assert hab ich eingebaut.
Das ist immer ein Problem (sorry, hatte den Parameter gar nicht gesehen
und daher nicht auf MT geschlossen).
Weißt Du überhaupt, was die anderen Threads machen?
Diese alten, zustandsbehafteten Funktionen sollte man überhaupt nicht
mehr benutzen. Die kommen aus einer Zeit, als Posix/*nix nur
heavyweight-processes kannte.
Oje schrieb:> Aha. Und das tut sie dann nur im Debugmodus zwangslaeufig nicht?
ein nicht vorhandener, aber notwendiger Returnwert ist UB.
UB kann alles bedeuten, auch ein (zufällig) nicht ganz falscher Wert im
(zufällig) richtigen Register, insofern ist es absolut müssig, darüber
zu spekulieren.
Hi,
ich würde gern hierüber nochmal diskutieren:
Wilhelm M. schrieb:> Da gibt es den Modulo-Trick:> sprintf(str, "%02d", 123456 % 100);
Sollte man es nicht besser gar nicht soweit kommen lassen? Was bringt
mir denn der kaputte Wert als String, außer woanders noch mehr unheil
anzurichten oder beim Debuggen/Konsolenausgabe Verwirrung zu stiften?
Meiner Meinung nach, sollte sowas eher vorher geprüft, entsprechend
behandelt (Exception/Fehlermeldung/Atomkrieg/etc.) und das sprintf gar
nicht erst ausgeführt werden.
René H. schrieb:> foobar schrieb:>> Es wurde schon genannt, dass gmtime ein NULL zurückliefern kann. Ein>> weiteres Problem angesichts des "ThreadControl" ist, dass gmtime nicht>> Thread-safe ist - dafür gibt es gmtime_r.>> Hmm..., das ist im Main Thread und sollte somit kein Problem darstellen.
In welchem Thread das läuft, ist egal. Sobald irgendein anderer Thread
zur gleichen Zeit gmtime() aufruft, hast du ein Problem. Wenn du
garantieren kannst, dass das niemals nie nicht ein anderer Thread tun
wird, dann ist alles ok.
Rolf M. schrieb:> In welchem Thread das läuft, ist egal. Sobald irgendein anderer Thread> zur gleichen Zeit gmtime() aufruft, hast du ein Problem. Wenn du> garantieren kannst, dass das niemals nie nicht ein anderer Thread tun> wird, dann ist alles ok.
Kann ich garantieren. Es werden 120 Thread gestartet und die berechnen
die Latency. Bei SIGTERM werden alle gestoppt und ein CSV geschrieben.
ABER: da fehlten einige return's. Ich habe alles korrigiert und werde
heute Abend mal testen.
Grüsse,
René
PS: Fcuk! Ich rufe localtime in den Threads auf. Das ist das Problem.
:-(
René H. schrieb:> ABER: da fehlten einige return's. Ich habe alles korrigiert und werde> heute Abend mal testen.
Wie oben schon gesagt: das ist der Grund.
>> PS: Fcuk! Ich rufe localtime in den Threads auf. Das ist das Problem.> :-(
Das ist zwar blöd, weil Du ggf. Dir Deinen Puffer zerstörst, aber es ist
nicht das Problem des SEGV. Versuch macht kluch.
Und schalt die Warnungen ein, und bau Zusicherungen ein oder
Fehlerbehandlung. So ist das jedenfalls nix.
Wilhelm M. schrieb:> Das ist zwar blöd, weil Du ggf. Dir Deinen Puffer zerstörst, aber es ist> nicht das Problem des SEGV.
Hinge das nicht von der Implementierung ab?
Wilhelm M. schrieb:> Heiko L. schrieb:>> Hinge das nicht von der Implementierung ab?>> Genau, deswegen steht da ggf.
Naja, der alte Buffer könnte ja auch free'd werden (je, nachdem in
welcher Referenz man nachschlägt).
Wilhelm M. schrieb:> Das ist zwar blöd, weil Du ggf. Dir Deinen Puffer zerstörst, aber es ist> nicht das Problem des SEGV.
Das kann ich ausschliessen und habe ich bedacht. Ich habe alles
hinzugefügt inkl. Fehlerhandling. Das brachte allerdings nichts :-(
strace sagt immer noch SIGSEV, es muss in den Threads liegen. Den Code
kann/darf ich leider nicht zeigen.
Grüsse,
René
René H. schrieb:> strace sagt immer noch SIGSEV, es muss in den Threads liegen. Den Code> kann/darf ich leider nicht zeigen.
Alles schon gemacht?
- warnungen an
- returns eingebaut
- *_r Funktionen
- Zusicherungen
- Datenstrukturen synchronisiert
René H. schrieb:> strace sagt immer noch SIGSEV, es muss in den Threads liegen. Den Code> kann/darf ich leider nicht zeigen.
Hast du die Info, wo genau? Ansonsten versuche mal valgrind oder einen
Sanitizer.
Wilhelm M. schrieb:> René H. schrieb:>> strace sagt immer noch SIGSEV, es muss in den Threads liegen. Den Code>> kann/darf ich leider nicht zeigen.>> Alles schon gemacht?>> - warnungen an> - returns eingebaut> - *_r Funktionen> - Zusicherungen>> - Datenstrukturen synchronisiert
Warnungen an sind Böse, ich habe es mal mit -Werror kompiliert. Das sind
mehrere 100'000 Zeilen Code, damit bin ich erst mal beschäftigt.
Return's sind drin. _r Funktionen auch.
Was heisst Zusicherung? Datenstrukturen sind synchronisiert.
Grüsse,
René
PS: der code ist über 15 Jahre alt :-(
René H. schrieb:> Was heisst Zusicherung?
Echt jetzt?
Assertionen.
In Deinem o.g. Code verwendest Du einen Datentyp (Zeigertyp), wobei Du
im folgenden Code davon ausgehst, dass ein bestimmter Wert (nullptr),
der ja zum Wertebereich gehört und von den Funktionen geliefert werden
kann, nicht vorkommt. So etwas darf man nicht machen. Um dies (Wert !=
nullptr) für den folgenden Code zuzusichern, verwendet man Assertionen.
Im Falle des Falles hast Du an genau der Zeile dann eben einen
Zusicherungsfehler (Exception in C++).
Die Assertionen sind Macros und nicht drin, wenn Du mit -DNDEBUG
compilierst.
René H. schrieb:> Datenstrukturen sind synchronisiert.
Wie?
René H. schrieb:> strace sagt immer noch SIGSEV,
valgrind wurde schon genannt, ich kann das nur noch einmal empfehlen.
https://valgrind.org/
Das findet und lokalisiert solche Fehler zuverlaesslich, auch in
multithreaded Code.
leo
Wilhelm M. schrieb:> Echt jetzt?>> Assertionen.
Dann sag doch gleich Assertions :-). Damit kann ich etwas machen, mit
dem Deutschsprachigen Begriff bin ich nicht vertraut.
Yep, sind drin.
Wilhelm M. schrieb:> Wie?
Jeder Thread hat seine eigene Datenstruktur, nach beenden der Threads
werden die eingesammelt. Da gibt es nichts zu synchronisieren. Ab 18:00
Uhr kann ich testen.
Wilhelm M. schrieb:> René H. schrieb:>> _r Funktionen auch.>> Beweise es!>> René H. schrieb:>> Datenstrukturen sind synchronisiert.>> René H. schrieb:>> Da gibt es nichts zu synchronisieren.>> Na was denn jetzt.
Mann, Wilhelm, ich darf den Teil vom Code hier nicht zeigen (ich würde
es liebend gern).
Es gibt nichts zu synchronisieren. Deshalb dass getLatencyMap. Die
Threads sammeln und nach Beendigung der Threads sammelt der Main Thread
alle maps ein und gibt sie als CSV aus. Jeder Thread ist für sich
autonom. Keine Mutex nichts.
René H. schrieb:> Mann, Wilhelm, ich darf den Teil vom Code hier nicht zeigen (ich würde> es liebend gern).
Den will ich gar nicht sehen.
Nur sollst Du zeigen, dass eben die zustandsbehaftenen Versionen der *_r
Funktionen nicht(!) vorkommen,
Wilhelm M. schrieb:> Nur sollst Du zeigen, dass eben die zustandsbehaftenen Versionen der *_r> Funktionen nicht(!) vorkommen,
Im gesamten Code, also die 100.000 Zeilen
Wilhelm M. schrieb:> also die
Mein Code sind lediglich ein paar hundert Zeilen und unabhängig davon.
Aber es ist nun mal Bestandteil vom "grossen" Projekt.
Oh ha,
liest Du eigentlich Doku?
Jetzt hast Du sicher einen SEGV.
Du hast das Prinzip einer thread-lokalen Datenstruktur gar nicht
verstanden. Ich befürchte schlimmes ...
Wilhelm M. schrieb:> Oh ha,>> liest Du eigentlich Doku?>> Jetzt hast Du sicher einen SEGV.>> Du hast das Prinzip einer thread-lokalen Datenstruktur gar nicht> verstanden. Ich befürchte schlimmes ...
Klär mich auf, bitte!
Wilhelm M. schrieb:> Du hast das Prinzip einer thread-lokalen Datenstruktur gar nicht> verstanden. Ich befürchte schlimmes ...
Eigentlich schon... wo liegt mein Fehler?
my_tm wird direkt zugewiesen, da sehe ich nicht was nicht initialisiert
sein soll.
auto kann ich nicht verwenden, wir haben noch c++98. Werfen also mit
Steinen ;-)
René H. schrieb:> my_tm wird direkt zugewiesen, da sehe ich nicht was nicht initialisiert> sein soll.
Ja, aber wo zeigt es denn hin, beim Aufruf von gmtime()???
Eher ins Nirvana!
René H. schrieb:> Also, jetzt ist die Methode definitiv Militärtauglich :-)
Nein, vorher war sie einfach nur falsch, jetzt ist sie (fast, s. Post
hierüber) richtig.
René H. schrieb:> Weil ich per se faul bin?
Dann ist es auch nicht militärtauglich ...
BTW: zeige, dass der restliche Code (100.00o Zeilen) , der nicht von Dir
ist, auch keine zustandsbehaftenen Funktionen in den Thread verwendet.
Markus F. schrieb:> Wilhelm M. schrieb:>> Jetzt hast Du sicher einen SEGV>> ... und das mit dem -Werror scheint auch eher unwahrscheinlichWilhelm M. schrieb:> Markus F. schrieb:>> ... und das mit dem -Werror scheint auch eher unwahrscheinlich>> Wo ist der Zusammenhang zu SEGV jetzt hier?
Wenn tatsächlich mit -Werror compiliert würde, gäb's keinen, weil kein
Binary.
René H. schrieb:> struct tm * my_tm = gmtime_r(&tv, my_tm);
wo soll gmtime_r() deiner Ansicht nach sein Ergebnis hinschreiben?
Tja, das hat der Author von QueueReader eine Beobachter-Funktion nicht
als solche gekennzeichnet.
Die Namensgebung (und das ggf. vergessene const) lassen evtl. auf einen
Java-Programmierer schließen.
Zudem können bei einem Zeiger zwei Sachen const sein: der
Zeiger(variable) kann read-only sein und/oder das Ziel. Du hast also
vier Möglichkeiten ...
Zeige mal die Schnittstelle von QueueReader.
Wilhelm M. schrieb:> Zudem können bei einem Zeiger zwei Sachen const sein: der> Zeiger(variable) kann read-only sein und/oder das Ziel. Du hast also> vier Möglichkeiten ...
Ja, das weiss ich auch ;-). Wenn man in dem Code mal mit const beginnt,
hat man ein halbes Jahr zu tun.
Und warum gibst Du einen Zeiger auf ein Datenmember als non-const-Ziel
heraus? Dann brauchst Du es ja gar nicht private zu machen!
Also: hier fehlen ja die elementaren Grundkenntnisse ... Einführungskurs
C++
René H. schrieb:> Wenn man in dem Code mal mit const beginnt,> hat man ein halbes Jahr zu tun.
Falsch. Man muss es von Anfang tun und das konsequent. Denn nur damit
ergibt sich das "Gebäude", was sich const-correctness nennt.
Wahllos mit dem Salzstreuer const verteilen, ist kein Design.
Du hast leider den Unterschied zwischen einer Beobachterfunktion und
Mutatorfunktion aka Modifier nicht verstanden (wahrscheinlich kommst Du
aus der Java-Welt).
Das schaut sehr nach Anfänger aus. Die Aufgabe, mit der Du betraut bist,
ist wohl eine Nummer zu groß für Dich im Moment.
Wilhelm M. schrieb:> n Unterschied zwischen einer Beobachterfunktion und> Mutatorfunktion aka M
Hab ich das?
Mein Studium liegt etwas zurück, und nein, ich bin kein Anfänger.
;-)
René H. schrieb:> Wilhelm M. schrieb:>> n Unterschied zwischen einer Beobachterfunktion und>> Mutatorfunktion aka M>> Hab ich das?>> Mein Studium liegt etwas zurück, und nein, ich bin kein Anfänger.>> ;-)
PS: ich kenne die Design Patterns der Gang of Four. Also, was soll ich
nicht begriffen haben?
PPS: Wie kommst Du eigentlich auf den Trichter das hier ein Observer
Pattern zugrunde liegt?
Wilhelm M. schrieb:> Falsch. Man muss es von Anfang tun und das konsequent. Denn nur damit> ergibt sich das "Gebäude", was sich const-correctness nennt.
Das sagt sich bei einem großen Haufen Legacy-Code immer ganz besonders
einfach.
Wilhelm M. schrieb:> Das schaut sehr nach Anfänger aus. Die Aufgabe, mit der Du betraut bist,> ist wohl eine Nummer zu groß für Dich im Moment.
Ich vermute mal sehr stark, dass René den Code nicht geschrieben hat und
der auch nicht im luftleeren Raum schwebt. Wenn man da mit -Werror
hantiert, dann fliegen einem relativ schnell auch andere Bestandteile um
die Ohren.
Zumindest bei meinem Arbeitgeber wäre der Ansatz "gut, dann repariere
ich den aufrufenden Code gleich mit" keine Alternative: Kein Mandat,
kein Projekt, kein Reviewer, keine Zeit.
René H. schrieb:> Also, was soll ich nicht begriffen haben?
Ich glaube, einfach nur die Welt der Theorie, in der Wilhelm lebt. :-)
Wilhelm M. schrieb:> eine Nummer zu groß für Dich im Moment.
Tja, das mag sein, an dem wächst man. Noch habe ich immer mein Ziel
erreicht und an dem habe ich nicht vor etwas zu ändern.
Schöner Code ist super, wenn man auf der schönen grünen Wiese beginnt.
Das ist hier nicht der Fall.
Ich komme übrigens nicht aus der Java Welt sondern aus der Objective
Pascal Welt.
Weshalb so überheblich? Du hast mir sehr viele gute Tipps gegeben die
ich sehr zu schätzen weiss.
René H. schrieb:> René H. schrieb:>> Wilhelm M. schrieb:>>> n Unterschied zwischen einer Beobachterfunktion und>>> Mutatorfunktion aka M>>>> Hab ich das?>>>> Mein Studium liegt etwas zurück, und nein, ich bin kein Anfänger.>>>> ;-)>> PS: ich kenne die Design Patterns der Gang of Four. Also, was soll ich> nicht begriffen haben?>> PPS: Wie kommst Du eigentlich auf den Trichter das hier ein Observer> Pattern zugrunde liegt?
Davon habe ich nichts geschrieben!
Deine Funktion ist immer noch non-const.
S. R. schrieb:> Ich glaube, einfach nur die Welt der Theorie, in der Wilhelm lebt. :-)
Danke! Ich habe schon begonnen an mir zu zweifeln ;-)
Man kann alles immer besser machen und ein Code-Review von aussen ist
nie schlecht.
Der Code stammt in der Tat nicht von mir, ich mache nur Maintenance.
Wilhelm M. schrieb:> Davon habe ich nichts geschrieben!> Deine Funktion ist immer noch non-const.
Ja, aber das ist nicht das Problem, erst mal Funktionalität. Dann kann
man es sauber machen.
Wilhelm M. schrieb:> Davon habe ich nichts geschrieben!Wilhelm M. schrieb:> Du hast leider den Unterschied zwischen einer Beobachterfunktion und
;-)
wie gesagt, wenn ich das mit dem const durchziehe, bin ich ein halbes
Jahr beschäftigt (und das bezahlt kein Mensch für "sauber").
René H. schrieb:> Wilhelm M. schrieb:>> Davon habe ich nichts geschrieben!>> Wilhelm M. schrieb:>> Du hast leider den Unterschied zwischen einer Beobachterfunktion und>>> ;-)>> wie gesagt, wenn ich das mit dem const durchziehe, bin ich ein halbes> Jahr beschäftigt (und das bezahlt kein Mensch für "sauber").
Es geht doch nicht um "sauber", sondern um richtig, jetzt und v.a. in
Zukunft. Rechne Deine Fehlersuchzeit dazu.
Btw. Const gab es schon vor c++98, und daher ist dieser Code wohl von
Anfang an konzeptionell falsch. Auch wenn er ggf. Irgendwie läuft. Ist
halt die Frage, was beim nä refactoring passiert.
Das sieht für mich nicht vertrauenserweckend aus. Heavy smell.
Wilhelm M. schrieb:> ht vertrauenserweckend aus. Heavy smell.
Der Code ist >20 Jahre alt und hat ein paar Millionen COL. Sowas baut
man nicht einfach mal schnell neu (daran haben sich schon viele die
Zähne ausgebissen und wurden schlussendlich gekündigt).
Es ist nicht das gelbe vom Ei, ist jetzt einfach so. Let's face it, das
ist eher der Standard als unüblich.
René H. schrieb:> Wilhelm M. schrieb:>> Sorry, das sind Anfängerthemen.>> Nein, sind es nicht!
Die Themen schon.
Leider nicht umgesetzt.
Wünsche Dir viel Glück mit dem Gebilde.
Wilhelm M. schrieb:> Es geht doch nicht um "sauber", sondern um richtig, jetzt und v.a. in> Zukunft. Rechne Deine Fehlersuchzeit dazu.>> Btw. Const gab es schon vor c++98, und daher ist dieser Code wohl von> Anfang an konzeptionell falsch. Auch wenn er ggf. Irgendwie läuft. Ist> halt die Frage, was beim nä refactoring passiert.>> Das sieht für mich nicht vertrauenserweckend aus. Heavy smell.
Naja, solange Du noch Geld an den Bankomaten bekommst .... ;-)
PS: Refactoring? Wovon träumst Du sonst noch so ....
PPS: der Code war ursprünglich C-Code. Dann machte man um die Headers
noch ein Class drum und dann war es C++.
Wilhelm M. schrieb:>> Ich glaube, einfach nur die Welt der Theorie, in der Wilhelm lebt. :-)> Sorry, das sind Anfängerthemen.
Das mag sein, aber es ist für die Realität unerheblich.
Wenn das ursprüngliche Design von einem Anfänger gemacht wurde (oder auf
einem antiken C++-Standard mit damals semi-kaputtem Compiler basiert),
dann hilft so eine Aussage eben nicht.
René H. schrieb:> PS: Refactoring? Wovon träumst Du sonst noch so ....>> PPS: der Code war ursprünglich C-Code. Dann machte man um die Headers> noch ein Class drum und dann war es C++.
Beide Aussagen wirken aus meiner beruflichen Erfahrung... vollkommen
korrekt. Danke. :-)
René H. schrieb:> Wilhelm M. schrieb:>> Es geht doch nicht um "sauber", sondern um richtig, jetzt und v.a. in>> Zukunft. Rechne Deine Fehlersuchzeit dazu.>>>> Btw. Const gab es schon vor c++98, und daher ist dieser Code wohl von>> Anfang an konzeptionell falsch. Auch wenn er ggf. Irgendwie läuft. Ist>> halt die Frage, was beim nä refactoring passiert.>>>> Das sieht für mich nicht vertrauenserweckend aus. Heavy smell.>> Naja, solange Du noch Geld an den Bankomaten bekommst .... ;-)>> PS: Refactoring? Wovon träumst Du sonst noch so ....>> PPS: der Code war ursprünglich C-Code. Dann machte man um die Headers> noch ein Class drum und dann war es C++.
Naja, jetzt verrennst du dich aber etwas. Du bist selbst nicht davon
überzeugt, dass der Code so sein sollte, reproduzierst aber eben diese
Qualität. Entweder du sagst "Sch**ß auf const. Ist nur Tipparbeit ohne
jeden Nutzen" oder du solltest wenigstens die Sachen, die du neu
schreibst, dem eigenen Maßstab folgend umsetzen.
Was Wilhelm mit const meint, ist, dass eine Funktion
1
struct X {
2
int fn() const;
3
};
(grob gesagt) die Zusicherung trifft, das X nicht zu verändern. Also
eine "nur lesende" oder beobachtende Funktion zu sein.
Weiter ausführend und den Begriff erweiternd, sollten Funktionen, die in
der Weise "const" gekennzeichnet sind, von mehreren Threads parallel
aufgerufen werden können, ohne dass Data-Races auftreten. Das dehnt den
Begriff in der Weise aus, dass auch keine unsynchronisierten
Seiteneffekte auftreten dürfen.
Problematisch im konkreten Fall ist, dass die Funktion selbst wieder ein
komplexes Objekt zurückliefert (die map). Der Devise folgend, dass
"const" genau die lesenden Funktionen markiert, die problemlos in
nebenläufigen Kontexten verwendet werden können, sollte eine solche
const-Funktion ein "const" Objekt zurückliefern. Also
1
const map& fn() const;
Da man aber ggf. in single-threaded Kontexten die map modifizieren will,
braucht man 2 overloads: einmal const einmal non-const
1
const map& fn() const;
2
map& fn();
Die Folge einer solchen "const-correctness" ist z.B., dass man anhand
der Deklarationen im Programm sofort sieht, wo Race-Conditions (nicht)
auftreten können. Ist aber einiges an Tipparbeit :)
Heiko L. schrieb:> Naja, jetzt verrennst du dich aber etwas. Du bist selbst nicht davon> überzeugt, dass der Code so sein sollte, reproduzierst aber eben diese> Qualität.
Das stimmt :-)
Heiko L. schrieb:> oder du solltest wenigstens die Sachen, die du neu> schreibst, dem eigenen Maßstab folgend umsetzen.
Sollte ich mal etwas neu Schreiben, halte ich bin daran. Das kannst Du
mir glauben.
Grüsse,
René
Wie Wilhelm schon angemerkt hat, const-correctness muss schon ab dem
Fundament gegeben sein. Damit kann man nicht nach >15 Jahren beginnen.
Es ist einfach nicht!
Grüsse
René
PS: Die Implementierung einer Klasse sollte eine BlackBox sein, wann
hast Du das letzte mal eine Klasse gesehen die man so verwenden kann?
S. R. schrieb:> Beide Aussagen wirken aus meiner beruflichen Erfahrung... vollkommen> korrekt. Danke. :-)
Gott sei Dank, ich dachte schon wir sind die einzige Frickel-Bude :-).
So, das Ganze läuft jetzt ohne SEGV durch, auch über Stunden ohne Memory
Leaks. Es waren die fehlenden returns (nicht das gmtime welches nicht
Threadsafe ist).
Ich danke allen die mir geholfen haben.
Grüsse,
René
René H. schrieb:> So, das Ganze läuft jetzt ohne SEGV durch, auch über Stunden ohne Memory> Leaks. Es waren die fehlenden returns
Na, das freut mich dann doch ;-)
René H. schrieb:> (nicht das gmtime welches nicht> Threadsafe ist).
Das war ja auch nur in Bezug auf Qualität und auf Deinen Begriff der
"militärtauglichkeit".
Trotzdem ist und bleibt das ein wichtiger Punkt. Auch Du solltest
spätestens jetzt anfangen, Deinen Code in hoher Qualität schreiben zu
wollen. Ein "das mache ich später" heißt ja in unserem Metier "nie".
Das bedeutet mindestens:
- Fehlerprüfung
- Zusicherungen
- Erweiterbarkeit
- Kapselung und Kohäsion
Und in diesem Zusammenhang ist gmtime() und / oder localtime() und
andere vergleichbare Sünden der C-Bibliothek oder POSIX ein no-go.