Hallo Forum,
folgenden Code benutze ich unter Embedded Linux (Archlinux ARM).
Bis zum fscanf geht alles wunderbar. In Target2 steht der Pfad zum File
drin.
Sobald fscanf() aufgerufen wurde stimmt der Zeiger auf Target2 nicht
mehr und das Programm stürzt bei free() ab.
Target2 pointer: vorher 0x00016b90 nachher 0x0001000
fscanf() liest aber den korrekten Wert aus und liefert wie erwartet 1
zurück.
Ist es ein Bug in fscanf() oder habe ich da irgendwas überlesen?
fscanfFehler schrieb:
Du hast einen Datentypfehler.
> ret = fscanf (TMP2, "%i", &val);
fscanf muss durch das %i davon ausgehen, dass sich hinter der Adresse,
die du ihm gibst (&val) sich ein int verbirgt.
Das tut es aber nicht
1
int8_tval=0;
auf deinem System ist höchst wahrscheinlich ein int8_t nicht dasselbe
wie ein int.
Damit schreibt dir fscanf beginnend mit der angegeben Adresse etwas in
den Speicher, was einem int entspricht. Nur: der reservierte Platz
reicht gar nicht dafür. D.h. fscanf überschreibt dir irgendwas im
Speicher, was hinter dieser Variablen im Speicher liegt.
Glücklicherweise war das ein Pointer, so dass du einen Fehler beim
free() bekommen hast. Dort hätte ja auch die Variable liegen können, die
steuert ob die Herz-Lungen-Maschine noch weiter laufen soll oder nicht,
und die im weiteren Programmlauf damit abgeschaltet würde. Ein Fehler
den du nicht sofort bemerkt hättest.
fscanfFehler schrieb:> Dachte das passt so da in dem GPIO File eh nur 1 oder 0 steht..
es wird aber immer die Variablenbreite geschrieben. Bei int sind das
mindestens 2byte.
fscanfFehler schrieb:> Es ist definiert:>> typedef signed char int8_t;
Schön.
INteressiert nur fscanf nicht.
GIbst du %i an, dann musst du die Adresse eines int an fscanf übergeben.
Punkt.
fscanf hat nur den Format-String um zu ergünden, was du da wohl für
Adressen angeben hast und wieviele Bytes daher dort beschrieben werden
müssen. Zwischen den %-Angaben im Format-String und den tatsächlichen
Argumenten muss es daher eine 100% Übereinstimmung geben. Jede %-Angabe
erfordert einen ganz bestimmten Datentyp (und nur diesen) in der
Argumentliste. Stimmen die nicht zusammen, dann macht es im besten Fall
'Peng'. Im schlimmsten Fall macht dein Programm Unsinn, der zunächst
unentdeckt bleibt.
Würde es was ausmachen wenn ich
int val;
dann fscanf()...
und dann
return((int_fast8_t)val);
mache?
Val kann ja wie gesagt nur 1 oder 0 enthalten. Daher müsste doch der
cast gehen?
Habe soweit möglich fast immer die gerade passende int größe verwendet..
Damit alles möglichst klein gehalten wird. int wäre auf dem System
32bit.
So hab ich bei allen rückgabewerten z.B.
int_fast8_t verwendet was signed char entspricht. Da die Rückgabewerte
meist nur -4 bis 4 sind..
fscanfFehler schrieb:> und dann> return((int_fast8_t)val);
ein cast würde ich vermeiden. Das ist immer nur eine Notlösung.
return val;
sollte schon reichen, maximal kommt eine Warnung.
dann könnte man es mit
return val&0xff;
machen.
fscanfFehler schrieb:> Würde es was ausmachen wenn ich> int val;> dann fscanf()...>> und dann> return((int_fast8_t)val);>> mache?
Das ist ok.
Dein Problem ist ja folgendes:
fscanf hat nur den Format-String um zu ergünden, was du da wohl für
Adressen angeben hast und wieviele Bytes daher dort beschrieben werden
müssen. Zwischen den %-Angaben im Format-String und den tatsächlichen
Argumenten muss es daher eine 100% Übereinstimmung geben. Jede %-Angabe
erfordert einen ganz bestimmten Datentyp (und nur diesen) in der
Argumentliste. Stimmen die nicht zusammen, dann macht es im besten Fall
'Peng'. Im schlimmsten Fall macht dein Programm Unsinn, der zunächst
unentdeckt bleibt.
Dein Problem steckt also in Annahmen, die fscanf machen muss, basierend
auf dem Format String. fscanf hat keine andere Möglichkeit als diese
'Glaskugel'. Daher musst du beim fscanf Aufruf die Dinge so einrichten,
dass die fscanf Glaskugel (deren Verhalten bekannt ist) zum richtigen
Schluss kommt bzw. du musst die Dinge so einrichten, dass du das
Verhalten von fscanf berücksichtigst, wenn du die Argumente
zusammenstellst.
fscanf passt sich nicht an dich an, sondern du musst dich an fscanf
anpassen.
Genau darum ist es auch so wichtig, zu WISSEN wie in C gewisse Dinge
funktionieren.
> Habe soweit möglich fast immer die gerade passende int größe verwendet..> Damit alles möglichst klein gehalten wird. int wäre auf dem System> 32bit.
Das ist alles löblich. Aber nicht die Ursache deines Problems.
fscanfFehler schrieb:> Okay, vielen Dank.>> Das bedeutet ich muss vorerst nur da nachschaun wo ich kein Standart-Int> verwende aber mit Standart-Funktionen arbeite.
Ganz genau. Und speziell dort, wo variadische Funktionen im SPiel sind,
wie zb printf oder scanf, die die Datentypen des Rests der Argumentliste
aus einem anderen Argument herleiten müssen (wie es scanf ja mit dem
Format String macht).
Bei anderen Funktionen hat der Compiler einen Prototyp und kann das
überwachen. Bei variadischen Funktionen hat er das aber nicht. Manche
Compiler prüfen zusätzlich und freundlicherweise den Formatstring gegen
die restlichen Angaben. Aber du kannst dich nicht darauf verlassen.
Daher ist es gut selbst die Regeln zu kennen.
Und PS: Standard schreibt sich mit 'D'. Das ist nicht die Kunst
(englisch: art) einen (Verkaufs-) Stand zu bauen und hat auch nichts mit
einer Standarte zu tun. Ich bin normal kein Rechtschreibnazi, aber
diesen Fehler sieht man leider allzuoft.
fscanfFehler schrieb:> Dann muss ich mal schaun ob ich so einen Fehler nicht noch woanders> habe...
Falls du den GCC benutzt, geht das ganz leicht: Der meckert dich nämlich
bei solchen Formatfehlern gerne an, wenn du ihn mit -Wformat (enthalten
in -Wall) darum bittest:
1
test.c:20:5: warning: format ‘%i’ expects argument of type ‘int *’, but argument 3 has type ‘int8_t *’ [-Wformat=]
2
ret = fscanf (TMP2, "%i", &val);
3
^
Korrekt müsste die Zeile so heißen:
1
ret=fscanf(TMP2,"%"SCNi8,&val);
Die SCN*-Makros gibt es für alle Integer-Typen und sind in inttypes.h
definiert. Entsprechend gibt es für printf die PRI*-Makros.
Wie ist es wenn ich system() benutze oder allgmein bei Funktionen die
normal int zurückgeben?
Ich lasse meist die Ausgabe auf int_fast8_t zurückgeben.
Muss ich hier auch aufpassen oder geht es hier?
fscanfFehler schrieb:> Muss ich hier auch aufpassen oder geht es hier?
es geht immer solange der Compiler den type sieht. Nur wenn du jemand
einen Zeiger ohne genauen Type übergibst, dann musst du aufpassen.
fscanfFehler schrieb:> diese Makros kannte ich noch gar nicht.>> Was machen die?
Sie expandieren zu den jeweils passenden scanf-Formaten. Bspw. ist SCNi8
auf meinem i83-PC als "hhi" definiert, so dass die obigen Zeile zu
1
ret=fscanf(TMP2,"%hhi",&val);
> Bzw. kann dann val bei int8_t bleiben?
Ja.
fscanfFehler schrieb:> Ich lasse meist die Ausgabe auf int_fast8_t zurückgeben.
Für int_fast8_t gibt es das Makro SCNiFAST8. Hier sind alle diese Makros
zusammengefasst (das # steht dabei für die Bitzahl, also 8, 16, 32 oder
64):
Der eine Fehler wurde ja hinreichend besprochen.
Korrekt also:
ret = (int8_t) fscanf (TMP2, "%i", &val);
Aber stehe ich irgendwie auf dem Schlauch?
Wieso macht man einen free() wenn man gar ein alloc() hat?
Kann so ein free() überhaupt funktionieren?
Ich behaupte mal nein.
Also ist es wohl so, daß die Funktion build_target einen Speicherbereich
alloziert und nur den Zeiger zurückgibt.
Richtig?
> Korrekt also:
Nope, die korrekte Lösung steht weiter oben.
> Wieso macht man einen free() wenn man gar ein alloc() hat?
Das zugehörige [m,c,re]alloc() steht hier offenkundig in build_target().
Ist schlechtes Design, aber es funktioniert(tm).
> Kann so ein free() überhaupt funktionieren?> Ich behaupte mal nein.
Doch[*]
HTH
[*] ..zumindest in bestimmten Fällen. Details dazu in der Doku.
Was wäre denn ein besseres Design??
Ich weiß das build_target() hier nicht so das gelbe vom ei ist ( besser
wäre snprintf oder so) aber die funktion habe ich eigentlich für etwas
anderes geschrieben aber hier konnte ich die einfach wieder verwenden
und sie macht das was sie soll...
Wie wäre sowas besser gelöst?
Vorteil an build_target es ist alles dynamisch durch malloc geregelt.
Man sieht nur leider nciht das man free verwenden muss am ende der
Funktion...
Vielleicht sollte ich es wirklich in etwas anderes abändern...
So muss ich auch 2 mal free machen..
mit snprintf nur einmal oder wenn ich ein festes Array mit [100] oder so
nehme passt auch alles rein...
Und das Array gibt es ja nur innerhalb der funktion..
Hm.
Ich bin ja immer wieder erstaunt wie sehr sich Leute von hinten ins knie
schießen für einen schritt nach vorn.
Wieso eigentlich nicht:
fopen Datei
Fread 1 byte
Fclose
If byte == '1'
Return 1
Else
Return 0
?
(Plus Fehlererkennung halt)
Anzahl benötigter alloc/free: 0
Weil sich die gpionum ändern kann...
Möchte die Funktion ja für jeden GPIO nutzen können.
Und ob ich jetzt fscanf oder fread aufrufe dürfte wohl keinen
unterschied machen oder?
dvbr schrieb:> (Plus Fehlererkennung halt)
Wenn du dir seinen Code mal genauer ansiehst, dann sind 80% des Codes
Fehlerbehandlung bzw. auf Fehlerbehandlung zurückzuführen, was an und
für sich nicht ungewöhnlich ist.
Man könnte den Code anders organisieren, so dass er sich nicht so
dermassen in die Länge zieht und auch optisch ansprechender aussieht.
Insbesondere könnte man alle 3 Code-Abschnitte mit den frees zu einem
einzigen zusammenfassen. Und auch sonst kann man noch so einiges
kritisieren, wie zb die Ausgabe von Fehlertexten (die in einem der
beiden Fälle dann noch nicht mal besonders hilfreich sind) in einer
Low-Level Funktion. So was haut einem jegliches User-Interface über den
Jordan. Aber das ist alles eine andere Geschichte.
Ob fscanf oder fgetc ist auch schon egal, zumal fscanf sich dann auch
gleich noch um das 'Problem' von Whitespace-Charactern (Spaces und
Zeilenumbrüche) kümmert, das du geflissentlich ignoriert hast. Genauso
wie die Behandlung von Lesefehlern bzw. EOF
Die Allokierung hat damit allerdings überhaupt nichts zu tun. Die
entsteht nur deshalb, weil er keine fixen Dateinamen hat, sondern die
zur Laufzeit zusammenstoppeln muss. Und da gibt es nun mal kein
Patentrezept. Entweder der Aufrufer der Funktion gibt den Speicher vor
(Problem: woher weiß er wie groß der sein muss), oder die Funktion
allokiert ihn dynamisch (Problem: der Aufrufer darf nicht auf den free
vergessen; Problem: dynamische Allokierung kann schief gehen)
Wie kann ich die denn ohne goto zusammenlegen?
Uns wurde beigebracht keine gotos!
Daher hab ich des öfteren so lange free Abschnitte...
Hab es jetzt auch umgebaut auf snprintf.
Muss also nur noch einmal Speicher anfordern und habe keine 2
Funktionsaufrufe mehr drin.
fscanfFehler schrieb:> Wie kann ich die denn ohne goto zusammenlegen?
Reicht dir ein Hinweis? :-)
Hier ist er: Überleg mal, wie dir die Einführung einer Variablen, die
den Returnwert der Funktion halten wird, es dir ermöglichen könnte, die
3 Freigabe und Return Sequenzen zusammenzulegen.
Achja wegen Fehler Meldung.
Das ganze läuft später auf einem headless System. Daher sind die
Meldungen nur für mich während ich meinen Code teste.
Und die Allokierung überprüfe ich auch immer schön und warte solange bis
er Speicher hat.
Werden spätere mehrere parallele State-Machines die mit
mutexen/semaphoren gesteuert werden, daher geht es wohl.
Hmm der Hinweis hat gereicht...
hätte man auch mal vorher drauf kommen können.... ;-)
Allerdings muss ich dann immer die Funktion bis dahin "ablaufen" und
kann nicht füher mit return rausspringen.
Hm muss ich mir mal überlegen ob ich es nochmal umbaua.
Habe an mehreren Stellen so lange free(), = NULL usw.
> Allerdings muss ich dann immer die Funktion bis dahin "ablaufen" und> kann nicht füher mit return rausspringen.
Warum sollte man das in diesem Fall auch wollen? So tief sind die
Schachtelungen der if noch nicht. :-)
fscanfFehler schrieb:> Uns wurde beigebracht keine gotos!
Uns auch... aber schau mal in Treiber/den Linux Kernel: das Zeug ist
voll von goto. goto ist gar nicht so boese wie immer alle sage.
Kaj schrieb:> fscanfFehler schrieb:>> Uns wurde beigebracht keine gotos!> Uns auch... aber schau mal in Treiber/den Linux Kernel: das Zeug ist> voll von goto. goto ist gar nicht so boese wie immer alle sage.
Wenn man weiß, was man tut, dann nicht.
Aber Neulinge wissen eben selten im Detail, was sie tun bzw. anrichten
Zumal der Linux Kernel etwas anderen Gesetzmässigkeiten unterliegt. Der
muss 'volle Kanne' laufen und da sehen x Leute über den Code drüber, ehe
er freigegeben wird.
Es gibt durchaus Fälle, in denen ein goto die einfachste und klarste
Lösung ist. Ist mir in 30 Jahren genau 1 mal untergekommen. Wobei man
durchaus über den Begriff 'klar' diskutieren kann.
Thread beobachten
Seitenaufteilung abschalten