Forum: Mikrocontroller und Digitale Elektronik unerklärlicher SIGSEGV


von René H. (Firma: anonymous) (hb9frh)


Lesenswert?

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?
1
/* operator<<:
2
 * ------------------------------------------------------------------------- */
3
std::ostream& operator<< (std::ostream& o, const ThreadControl& c)
4
{
5
   latency_map::const_iterator it;
6
   readerMap_t::const_iterator itm;
7
8
   
9
   for (itm = c.reader_map_.begin(); itm != c.reader_map_.end(); ++itm) {
10
      QueueReader*   qr = itm->second.q_reader;
11
      latency_map*   map = qr->getLatencyMap();
12
13
      for (it = map->begin(); it != map->end(); ++it) {
14
         uint32_t ts = it->first;
15
         const time_t   tv = (time_t)ts;
16
         struct tm *my_tm = gmtime(&tv);
17
         char     timeStr[32];
18
         char     dateStr[32];
19
20
         sprintf(dateStr, "%02d.%02d.%04d", my_tm->tm_mday, my_tm->tm_mon+1, my_tm->tm_year+1900);
21
         sprintf(timeStr, "%02d:%02d:%02d", my_tm->tm_hour, my_tm->tm_min, my_tm->tm_sec);
22
23
         o << (uint16_t) itm->first << ";";
24
         o << dateStr << " " << timeStr << ";";
25
26
         o << it->second << std::endl;
27
      }
28
   }
29
}

Es muss ein Speicherproblem sein, aber ich sehe nicht wo. Laufzeit kann 
ich ausschliessen.

Grüsse,
René

von Heiko L. (zer0)


Lesenswert?

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.

von Wilhelm M. (wimalopaan)


Lesenswert?

Ja, wo ist denn das return o;

Außerdem: assert(my_tm != nullptr);

von René H. (Firma: anonymous) (hb9frh)


Lesenswert?

Wilhelm M. schrieb:
> Ja, wo ist denn das return o;
>
> Außerdem: assert(my_tm != nullptr);

Aaaargh!!! Thx.

Grüsse,
René

von René H. (Firma: anonymous) (hb9frh)


Lesenswert?

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.

von Heiko L. (zer0)


Lesenswert?

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.

von René H. (Firma: anonymous) (hb9frh)


Lesenswert?

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.

von Heiko L. (zer0)


Lesenswert?

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

von Wilhelm M. (wimalopaan)


Lesenswert?

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?

von René H. (Firma: anonymous) (hb9frh)


Lesenswert?

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é

von Wilhelm M. (wimalopaan)


Lesenswert?

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.

von Heiko L. (zer0)


Lesenswert?

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

von René H. (Firma: anonymous) (hb9frh)


Lesenswert?

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.

von René H. (Firma: anonymous) (hb9frh)


Lesenswert?

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

von Heiko L. (zer0)


Lesenswert?

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?

von Wilhelm M. (wimalopaan)


Lesenswert?

René H. schrieb:
> Nope, kann es nicht. Sonst zeig mir wie .....

Sicher:
1
printf("%02d\n", 12345678);

von René H. (Firma: anonymous) (hb9frh)


Lesenswert?

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é

von René H. (Firma: anonymous) (hb9frh)


Lesenswert?

Ich denke aber dein Hinweis wird es gewesen sein, leider kann ich erst 
ab morgen Abend testen da kein Input übers WE.

von René H. (Firma: anonymous) (hb9frh)


Lesenswert?

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é

von René H. (Firma: anonymous) (hb9frh)


Lesenswert?

Wilhelm M. schrieb:
> René H. schrieb:
>> Nope, kann es nicht. Sonst zeig mir wie .....
>
> Sicher:
> printf("%02d\n", 12345678);

Mist!!! Ihr habt Recht :-(
1
int main(int argc, char** argv)
2
{
3
   char str[32];
4
5
   sprintf(str, "%02d", 123456);
6
7
   std::cout << str << std::endl;
8
   
9
   return(0);
10
}
1
xxx@yyyyyy:~/tmp> ./overwrite 
2
123456

Grüsse,
René

von Wilhelm M. (wimalopaan)


Lesenswert?

René H. schrieb:
> sprintf(str, "%02d", 123456);

Da gibt es den Modulo-Trick:
1
sprintf(str, "%02d", 123456 % 100);

von René H. (Firma: anonymous) (hb9frh)


Lesenswert?

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é

von Wilhelm M. (wimalopaan)


Lesenswert?

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

von Heiko L. (zer0)


Lesenswert?

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.

von René H. (Firma: anonymous) (hb9frh)


Lesenswert?

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

von foobar (Gast)


Lesenswert?

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.

von René H. (Firma: anonymous) (hb9frh)


Lesenswert?

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.

von Wilhelm M. (wimalopaan)


Lesenswert?

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.

von Oje (Gast)


Lesenswert?

Ich dieses Verhalten schon oft erlebt und tippe auf was anderes: Eine 
uninitialisierte Variable in deinem Programm...

von Markus F. (mfro)


Lesenswert?

da gibt's doch nix rumzudeuteln.

Wenn man << überlädt und keinen vernünftigen Returnwert zurückgibt, muß 
die (meist folgende) Verkettung:
1
std::cout << obj << irgendwas;

praktisch zwangsläufig auf die Nase fallen. Die ist auf einen validen 
Returnwert angewiesen.

von Oje (Gast)


Lesenswert?

Aha. Und das tut sie dann nur im Debugmodus zwangslaeufig nicht?

von Markus F. (mfro)


Lesenswert?

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.

von test (Gast)


Lesenswert?

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.

von Wilhelm M. (wimalopaan)


Lesenswert?

Brauchen wir nicht. Steht gaaaanz oben.

von Rolf M. (rmagnus)


Lesenswert?

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.

von René H. (Firma: anonymous) (hb9frh)


Lesenswert?

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. 
:-(

von Wilhelm M. (wimalopaan)


Lesenswert?

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.

von Heiko L. (zer0)


Lesenswert?

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?

von Wilhelm M. (wimalopaan)


Lesenswert?

Heiko L. schrieb:
> Hinge das nicht von der Implementierung ab?

Genau, deswegen steht da ggf.

von Heiko L. (zer0)


Lesenswert?

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

von René H. (Firma: anonymous) (hb9frh)


Lesenswert?

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é

von Wilhelm M. (wimalopaan)


Lesenswert?

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

von Heiko L. (zer0)


Lesenswert?

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.

von René H. (Firma: anonymous) (hb9frh)


Lesenswert?

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 :-(

von Wilhelm M. (wimalopaan)


Lesenswert?

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?

von leo (Gast)


Lesenswert?

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

von René H. (Firma: anonymous) (hb9frh)


Lesenswert?

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.

von Wilhelm M. (wimalopaan)


Lesenswert?

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.

von René H. (Firma: anonymous) (hb9frh)


Lesenswert?

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.

von Wilhelm M. (wimalopaan)


Lesenswert?

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,

von René H. (Firma: anonymous) (hb9frh)


Lesenswert?

Wilhelm M. schrieb:
> Beweise es!
1
/* operator<<:
2
 * ------------------------------------------------------------------------- */
3
std::ostream& operator<< (std::ostream& o, const ThreadControl& c)
4
{
5
   latency_map::const_iterator it;
6
   readerMap_t::const_iterator itm;
7
8
   
9
   for (itm = c.reader_map_.begin(); itm != c.reader_map_.end(); ++itm) {
10
      QueueReader*   qr = itm->second.q_reader;
11
      assert (qr != nullptr);
12
13
      latency_map*   map = qr->getLatencyMap();
14
      assert (map != nullptr);
15
16
      for (it = map->begin(); it != map->end(); ++it) {
17
         uint32_t ts = it->first;
18
         const time_t  tv = (time_t)ts;
19
         struct tm * my_tm = gmtime(&tv);
20
         char     timeStr[32];
21
         char     dateStr[32];
22
23
         assert (my_tm != nullptr);
24
         sprintf(dateStr, "%02d.%02d.%04d", my_tm->tm_mday, my_tm->tm_mon+1, my_tm->tm_year+1900);
25
         sprintf(timeStr, "%02d:%02d:%02d", my_tm->tm_hour, my_tm->tm_min, my_tm->tm_sec);
26
27
         o << (uint16_t) itm->first << ";";
28
         o << dateStr << " " << timeStr << ";";
29
30
         o << it->second << std::endl;
31
      }
32
   }
33
34
   return(o);
35
}

von Wilhelm M. (wimalopaan)


Lesenswert?

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

von Wilhelm M. (wimalopaan)


Lesenswert?

Da sehe ich immer noch gmtime()

von René H. (Firma: anonymous) (hb9frh)


Lesenswert?

So, Korrektur:
1
/* operator<<:
2
 * ------------------------------------------------------------------------- */
3
std::ostream& operator<< (std::ostream& o, const ThreadControl& c)
4
{
5
   latency_map::const_iterator it;
6
   readerMap_t::const_iterator itm;
7
8
   
9
   for (itm = c.reader_map_.begin(); itm != c.reader_map_.end(); ++itm) {
10
      QueueReader*   qr = itm->second.q_reader;
11
      assert (qr != nullptr);
12
13
      latency_map*   map = qr->getLatencyMap();
14
      assert (map != nullptr);
15
16
      for (it = map->begin(); it != map->end(); ++it) {
17
         uint32_t ts = it->first;
18
         const time_t  tv = (time_t)ts;
19
         struct tm * my_tm = gmtime_r(&tv, my_tm);
20
         char     timeStr[32];
21
         char     dateStr[32];
22
23
         assert (my_tm != nullptr);
24
         sprintf(dateStr, "%02d.%02d.%04d", my_tm->tm_mday, my_tm->tm_mon+1, my_tm->tm_year+1900);
25
         sprintf(timeStr, "%02d:%02d:%02d", my_tm->tm_hour, my_tm->tm_min, my_tm->tm_sec);
26
27
         o << (uint16_t) itm->first << ";";
28
         o << dateStr << " " << timeStr << ";";
29
30
         o << it->second << std::endl;
31
      }
32
   }
33
34
   return(o);
35
}

von René H. (Firma: anonymous) (hb9frh)


Lesenswert?

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.

von Wilhelm M. (wimalopaan)


Lesenswert?

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

von René H. (Firma: anonymous) (hb9frh)


Lesenswert?

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!

von Markus F. (mfro)


Lesenswert?

Wilhelm M. schrieb:
> Jetzt hast Du sicher einen SEGV

... und das mit dem -Werror scheint auch eher unwahrscheinlich

von René H. (Firma: anonymous) (hb9frh)


Lesenswert?

Ich kann beim besten Willen nicht sehen wo da ein SEGV sein soll.

von Wilhelm M. (wimalopaan)


Lesenswert?

René H. schrieb:
> Klär mich auf, bitte!

1
struct tm my_tm;
2
auto result = gmtime_r(&tv, &my_tm);

von Wilhelm M. (wimalopaan)


Lesenswert?

René H. schrieb:
> Ich kann beim besten Willen nicht sehen wo da ein SEGV sein soll.

uninitialisierter Zeiger my_tm;

von Wilhelm M. (wimalopaan)


Lesenswert?

Markus F. schrieb:
> ... und das mit dem -Werror scheint auch eher unwahrscheinlich

Wo ist der Zusammenhang zu SEGV jetzt hier?

von René H. (Firma: anonymous) (hb9frh)


Lesenswert?

Wilhelm M. schrieb:
> Du hast das Prinzip einer thread-lokalen Datenstruktur gar nicht
> verstanden. Ich befürchte schlimmes ...

Eigentlich schon... wo liegt mein Fehler?

von Wilhelm M. (wimalopaan)


Lesenswert?

René H. schrieb:
> Eigentlich schon... wo liegt mein Fehler?

s.a. mein Post gerade eben

von René H. (Firma: anonymous) (hb9frh)


Lesenswert?

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

von Wilhelm M. (wimalopaan)


Lesenswert?

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!

von René H. (Firma: anonymous) (hb9frh)


Lesenswert?

so ist dann besser :-)
1
/* operator<<:
2
 * ------------------------------------------------------------------------- */
3
std::ostream& operator<< (std::ostream& o, const ThreadControl& c)
4
{
5
   latency_map::const_iterator it;
6
   readerMap_t::const_iterator itm;
7
8
   
9
   for (itm = c.reader_map_.begin(); itm != c.reader_map_.end(); ++itm) {
10
      QueueReader*   qr = itm->second.q_reader;
11
      assert (qr != nullptr);
12
13
      latency_map*   map = qr->getLatencyMap();
14
      assert (map != nullptr);
15
16
      for (it = map->begin(); it != map->end(); ++it) {
17
         uint32_t ts = it->first;
18
         const time_t  tv = (time_t)ts;
19
         struct tm  my_tm;
20
         gmtime_r(&tv, &my_tm);
21
         char     timeStr[32];
22
         char     dateStr[32];
23
24
         assert (my_tm != nullptr);
25
         sprintf(dateStr, "%02d.%02d.%04d", my_tm.tm_mday, my_tm.tm_mon+1, my_tm.tm_year+1900);
26
         sprintf(timeStr, "%02d:%02d:%02d", my_tm.tm_hour, my_tm.tm_min, my_tm.tm_sec);
27
28
         o << (uint16_t) itm->first << ";";
29
         o << dateStr << " " << timeStr << ";";
30
31
         o << it->second << std::endl;
32
      }
33
   }
34
35
   return(o);
36
}

von René H. (Firma: anonymous) (hb9frh)


Lesenswert?

Also, jetzt ist die Methode definitiv Militärtauglich :-)

von Wilhelm M. (wimalopaan)


Lesenswert?

Nein!
Rückgabewert von gmtime_r() checken.

von Wilhelm M. (wimalopaan)


Lesenswert?

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.

von René H. (Firma: anonymous) (hb9frh)


Lesenswert?

1
/* operator<<:
2
 * ------------------------------------------------------------------------- */
3
std::ostream& operator<< (std::ostream& o, const ThreadControl& c)
4
{
5
   latency_map::const_iterator it;
6
   readerMap_t::const_iterator itm;
7
8
   
9
   for (itm = c.reader_map_.begin(); itm != c.reader_map_.end(); ++itm) {
10
      QueueReader*   qr = itm->second.q_reader;
11
      assert (qr != nullptr);
12
13
      latency_map*   map = qr->getLatencyMap();
14
      assert (map != nullptr);
15
16
      for (it = map->begin(); it != map->end(); ++it) {
17
         uint32_t ts = it->first;
18
         const time_t  tv = (time_t)ts;
19
         struct tm  my_tm, *result;
20
         result = gmtime_r(&tv, &my_tm);
21
         char     timeStr[32];
22
         char     dateStr[32];
23
24
         assert (result != nullptr);
25
         sprintf(dateStr, "%02d.%02d.%04d", my_tm.tm_mday, my_tm.tm_mon+1, my_tm.tm_year+1900);
26
         sprintf(timeStr, "%02d:%02d:%02d", my_tm.tm_hour, my_tm.tm_min, my_tm.tm_sec);
27
28
         o << (uint16_t) itm->first << ";";
29
         o << dateStr << " " << timeStr << ";";
30
31
         o << it->second << std::endl;
32
      }
33
   }
34
35
   return(o);
36
}

Militärtauglich?

von Wilhelm M. (wimalopaan)


Lesenswert?

Warum sind qr, ts, result und map non-const?

von René H. (Firma: anonymous) (hb9frh)


Lesenswert?

Weil ich per se faul bin?

von Wilhelm M. (wimalopaan)


Lesenswert?

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.

von Markus F. (mfro)


Lesenswert?

Markus F. schrieb:
> Wilhelm M. schrieb:
>> Jetzt hast Du sicher einen SEGV
>
> ... und das mit dem -Werror scheint auch eher unwahrscheinlich

Wilhelm 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?

von Wilhelm M. (wimalopaan)


Lesenswert?

Markus F. schrieb:
> wo soll gmtime_r() deiner Ansicht nach sein Ergebnis hinschreiben?

Hatten wir schon geklärt ;-)

von Wilhelm M. (wimalopaan)


Lesenswert?

Markus F. schrieb:
> Wenn tatsächlich mit -Werror compiliert würde, gäb's keinen, weil kein
> Binary.

Ja, Code der nicht compiliert, ist guter Code.

von René H. (Firma: anonymous) (hb9frh)


Lesenswert?

Mein Compiler ist const feindlich ;-)
1
/home/xxx/Proj/yyyy/tools/latency_measure/thread_control.cpp:82:44: error: passing \u2018const QueueReader\u2019 as \u2018this\u2019 argument of \u2018latency_map* QueueReader::getLatencyMap()\u2019 discards qualifiers [-fpermissive]
2
       latency_map* map = qr->getLatencyMap();

von René H. (Firma: anonymous) (hb9frh)


Lesenswert?

so ....
1
/* operator<<:
2
 * ------------------------------------------------------------------------- */
3
std::ostream& operator<< (std::ostream& o, const ThreadControl& c)
4
{
5
   latency_map::const_iterator it;
6
   readerMap_t::const_iterator itm;
7
8
   
9
   for (itm = c.reader_map_.begin(); itm != c.reader_map_.end(); ++itm) {
10
      QueueReader* const qr = itm->second.q_reader;
11
      assert (qr != nullptr);
12
13
      latency_map* const map = qr->getLatencyMap();
14
      assert (map != nullptr);
15
16
      for (it = map->begin(); it != map->end(); ++it) {
17
         const uint32_t ts = it->first;
18
         const time_t  tv = (time_t)ts;
19
         struct tm  my_tm;
20
         const  struct tm *result;
21
         result = gmtime_r(&tv, &my_tm);
22
         char     timeStr[32];
23
         char     dateStr[32];
24
25
         assert (result != nullptr);
26
         sprintf(dateStr, "%02d.%02d.%04d", my_tm.tm_mday, my_tm.tm_mon+1, my_tm.tm_year+1900);
27
         sprintf(timeStr, "%02d:%02d:%02d", my_tm.tm_hour, my_tm.tm_min, my_tm.tm_sec);
28
29
         o << (uint16_t) itm->first << ";";
30
         o << dateStr << " " << timeStr << ";";
31
32
         o << it->second << std::endl;
33
      }
34
   }
35
36
   return(o);
37
}

Jetzt Militärtauglich?

von Wilhelm M. (wimalopaan)


Lesenswert?

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.

von René H. (Firma: anonymous) (hb9frh)


Lesenswert?

1
class QueueReader
2
{
3
   public:
4
      QueueReader();
5
      virtual ~QueueReader();
6
7
      void  process (QueueData* q_data);
8
      bool           finish;
9
      inline latency_map *getLatencyMap(void) { return(&latency_map_); };
10
   protected:
11
12
   private:
13
      latency_map    latency_map_;
14
};

hmmm ....

von René H. (Firma: anonymous) (hb9frh)


Lesenswert?

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.

von Wilhelm M. (wimalopaan)


Lesenswert?

Warum ist die Beobachterfunktion non-const?

von Wilhelm M. (wimalopaan)


Lesenswert?

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++

von Wilhelm M. (wimalopaan)


Lesenswert?

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.

von René H. (Firma: anonymous) (hb9frh)


Lesenswert?

René H. schrieb:
> inline latency_map *getLatencyMap(void) { return(&latency_map_);
> };

jetzt ist sie const, noch conster geht nicht :-)
1
      inline const latency_map *const getLatencyMap(void) { return(&latency_map_); };

von Wilhelm M. (wimalopaan)


Lesenswert?

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.

von René H. (Firma: anonymous) (hb9frh)


Lesenswert?

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.

;-)

von René H. (Firma: anonymous) (hb9frh)


Lesenswert?

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?

von S. R. (svenska)


Lesenswert?

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. :-)

von René H. (Firma: anonymous) (hb9frh)


Lesenswert?

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.

von Wilhelm M. (wimalopaan)


Lesenswert?

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.

von René H. (Firma: anonymous) (hb9frh)


Lesenswert?

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.

von René H. (Firma: anonymous) (hb9frh)


Lesenswert?

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.

von René H. (Firma: anonymous) (hb9frh)


Lesenswert?

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").

von Wilhelm M. (wimalopaan)


Lesenswert?

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.

von Wilhelm M. (wimalopaan)


Lesenswert?

S. R. schrieb:
> Ich glaube, einfach nur die Welt der Theorie, in der Wilhelm lebt. :-)

Sorry, das sind Anfängerthemen.

von René H. (Firma: anonymous) (hb9frh)


Lesenswert?

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.

von René H. (Firma: anonymous) (hb9frh)


Lesenswert?

Wilhelm M. schrieb:
> Sorry, das sind Anfängerthemen.

Nein, sind es nicht!

von René H. (Firma: anonymous) (hb9frh)


Lesenswert?

Wilhelm M. schrieb:
> Es geht doch nicht um "sauber", sondern um richtig,

Richtig ist, wenn es das macht was es muss. Der Rest ist sauber!

von Wilhelm M. (wimalopaan)


Lesenswert?

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.

von René H. (Firma: anonymous) (hb9frh)


Lesenswert?

Wilhelm M. schrieb:
> Die Themen schon.
> Leider nicht umgesetzt.

Stimmt. ;-)

von René H. (Firma: anonymous) (hb9frh)


Lesenswert?

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

von S. R. (svenska)


Lesenswert?

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. :-)

von Heiko L. (zer0)


Lesenswert?

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

von René H. (Firma: anonymous) (hb9frh)


Lesenswert?

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é

von René H. (Firma: anonymous) (hb9frh)


Lesenswert?

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?

von René H. (Firma: anonymous) (hb9frh)


Lesenswert?

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 :-).

von René H. (Firma: anonymous) (hb9frh)


Lesenswert?

@Mods

Sorry, aber das Thema passt jetzt nun wirklich nicht in dieses 
Unterforum.

von René H. (Firma: anonymous) (hb9frh)


Lesenswert?

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é

von Wilhelm M. (wimalopaan)


Lesenswert?

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.

Bitte melde dich an um einen Beitrag zu schreiben. Anmeldung ist kostenlos und dauert nur eine Minute.
Bestehender Account
Schon ein Account bei Google/GoogleMail? Keine Anmeldung erforderlich!
Mit Google-Account einloggen
Noch kein Account? Hier anmelden.