Function-Pointer Problem nach Stromunterbrechung

[Peter S. (petershaw)]

Hallo,

ich sitze seit Tagen an einem merkwürdigen Problem und komme keinen 
Schritt weiter.
Ich habe ein kleines Gerät gebaut das mir als Messgerät dienen soll. 
Momentan schraube ich gerade an einer Firmeware. Diese findet ihr in der 
momentanen Version auf GitHub:
https://github.com/petershaw/AnalyzerDude

Wenn ich den Programmer an das Gerät stecke und die Firmeware flashe ist 
alles in Ordnung. Ich kann durch meine Menüs gehen und die hinterlegten 
Funktionen aufrufen. Wenn ich aber den Stromkreislauf einmal unterbrache 
und dann wieder belebe, kann ich keine Funktionen mehr aufrufen... Wobei 
das blättern im Menü noch funktioniert.

Da sich das etwas schwierig erklären lässt, habe ich ein kleines Video 
angefertigt. Sorry wegen Sprache ich versuche das momentan wieder besser 
in den Griff zu bekommen...

http://www.youtube.com/watch?v=dudqfP8_SjA

Bitte schaut euch die 5min mal an, um jeden Hinweis bin ich echt 
dankbar. Ich weiß nicht was ich noch versuchen soll.

Danke,
Peter

[Falk B. (falk)]

Klingt nach einem Initialisierungsfehler.

[reimay (Gast)]

Hatte mal ein ähnliches Problem.
Bei mir lag es an der Resetbeschaltung.

[Helmut L. (helmi1)]

Wenn du schon Malloc benutzt um damit Speicher zu beantragen wo 
ueberpruefst du ob du den Speicher auch bekommen hast b.z.w. wo ist die 
Initialisierung fuer Malloc?

[Peter S. (petershaw)]

hm, ja. Da hast du recht, aber warum funzt das wenn der Programmer noch 
verbunden ist?

[Falk B. (falk)]

Möglicherweise macht dein Programmer einen sauberen Reset, NACHDEM die 
Versorgungsspannung stabil steht. Kanst du leicht prüfen. Schalte dein 
Gerät ein und mach per Jumper/Taster einen Harwarereset.

Wenn das passt -> Broun Out Detektor einschalten

[Helmut L. (helmi1)]

Peter Shaw schrieb:
> Da hast du recht, aber warum funzt das wenn der Programmer noch
> verbunden ist?

Eventuell ist der Heap da einigermassen initialisiert das es 
funktioniert.

[Thomas E. (thomase)]

Peter Shaw schrieb:
> Ich weiß nicht was ich noch versuchen soll.

//initialize the device
device_init();
_delay_ms(1000);   // wait for 1sec
device_reset();

Vielleicht hilft ein Delay VOR der Initialisierung. Möglicherweise ist 
der Displaycontroller noch nicht so weit, wenn der AVR loslegt.

Helmut Lenzen schrieb:
> Eventuell ist der Heap da einigermassen initialisiert das es
> funktioniert.

Das würde aber auch bedeuten, daß der Compiler totale Scheiße gebaut 
hat.

mfg.

[Karl H. (kbuchegg)]

In deinem Code wird mir da ehrlich gesagt viel zu viel rumgecastet. Und 
die vielen void Pointer tun da ein übriges. In so einem Code sollte es 
eigentlich keinen einzigen Cast geben. Zumindest nicht im Zusammenhang 
mit Funktionspointern. Was du da machst, das ist aktive Sabotage des 
Typ-Systems mit dem dich der Compiler auf Datentypfehler aufmerksam 
macht.

Behandle Casts wie Waffen! Du willst sie nicht im Haus haben, wenn es 
auch nur irgendwie anders geht!

Die Aufteilung in die vielen Files macht es nicht einfach, da den 
Überblick zu behalten, wo welcher Cast nix anstellt und welche Casts 
eigentlich notwendig sind.

[Thomas E. (thomase)]

Karl Heinz Buchegger schrieb:
> Du willst sie nicht im Haus haben, wenn es
> auch nur irgendwie anders geht!
Und wenn er Amerikaner ist(Peter Shaw)?

Karl Heinz Buchegger schrieb:
> In deinem Code wird mir da ehrlich gesagt viel zu viel rumgecastet. Und
> die vielen void Pointer tun da ein übriges.
Aber das ist doch keine Erklärung dafür, daß der Controller je nach 
Resetart einmal Hü und das andere mal Hott macht.

mfg.

[Karl H. (kbuchegg)]

Thomas Eckmann schrieb:

> Aber das ist doch keine Erklärung dafür, daß der Controller je nach
> Resetart einmal Hü und das andere mal Hott macht.

Nein ist es nicht.
War mehr so eine generelle Anmerkung.

So wie sich das sehe, sind alle Variablen soweit initialisiert. Den Code 
zum anhängen eines Menüs hab ich durch, der dürfte ok sein. Den Code für 
die Submenüs hab ich noch nicht durch.

Ich such immer noch nach einer uninitalisierten Variablen, die nach 
einem Chip-Erase einen 0-Wert hat, die aber nach einem Reset bereits 
einen Wert im SRAM vorfindet, wodurch alles den Bach runtergeht. Nur: 
Bis jetzt hab ich noch nichts.

[Karl H. (kbuchegg)]

Moment mal.

Ein Menu Objekt besteht aus den Pointern

   next
   parent
   submenu
   data
   fn


Seh ich mir zb deine menu_init an

void ui_menu_init(void){
    menu = malloc(sizeof(menu_t));
    menuentry_t *entry = malloc(sizeof (menuentry_t));
    entry->position = -1;
    
    entry->data = "NO ENTRY";

    // loop around
    entry->next = entry;
    
    menu->firstentry = entry;
    menu->lastentry = entry;
    
    active = entry;
}

dann wird da alles mögliche gemacht, aber von den Pointern werden gerade 
mal 2 intialisiert.

malloc() liefert keinen initialisierten Speicher! Was immer da im 
Speicher an Bytes vorhanden ist, du erbst es!

Du hast 2 Möglichkeiten: entweder du benutzt calloc
oder aber - was ich gerne tue - du schiebst die Allokierung samt 
Initialisierung in eine eigen Funktion

menuentry_t *ui_menu_allocEntry()
{
  menuentry_t *newEntry = malloc( sizeof menuentry_t );

  if( newEntry ) {
    newEntry->next    = NULL;
    newEntry->parent  = NULL;
    newEntry->submenu = NULL;
    newEntry->data    = NULL;
    newEntry->fn      = NULL;
  }

  return newEntry;
}

und benuztzt ausschliessliche diese Funktion in menu_init bzw. den 
diversen Add Funktion um einen neuen Entry zu erzeugen. Aber du kannst 
es dir bei komplexeren System nicht leisten, da uninitialisierte Pointer 
rumliegen zu haben, von denen du nicht mit 100% Sicherheit weißt, dass 
sie NULL sind.


Der Unterschied, ohne jetzt den restlichen Code zu Ende analysiert zu 
haben, kann sehr wohl genau den Unterschied ausmachen, den du siehst. 
Wenn du den µC neu flasht dann wird ein Chip-Erase gamcht. Danach hat 
der SRAM lauter 0-Bytes. Aber die hat er nicht mehr, wenn dein Programm 
schon mal gelaufen ist und du den µC resettest bzw. den Strom aufdrehst. 
Dann steht irgendwas im Speicher.

Millionen Programmabstürze gehen auf das Konto von nicht initialisierten 
Variablen.

[Peter S. (petershaw)]

Sorry wegen der vielen Files. Im XCode ist das schön übersichtlich. 
Jedenfalls für meine Arbeitsweise.

Ich hab in meinem Testcode die Submenüs auch mal rausgenommen.

Das mit den Casts nehme ich mit zu herzen - da wo ich herkomme benutzt 
man diese (viid *) zur Leserlichkeit (um im Aufrug zu sehen was unten 
Sache ist). Also wenn ich eine void *fn_hello(void) habe und diese im 
Aufruf einer Funktion mitgebe hatte ich mal gelernt aus diesem grung 
nicht call(fn_hello()) anzugeben, sondern hier den Wert zu "versichern" 
call((void *)fn_hello)

Danke für eure Mühe, ich wäre echt überglücklich hier ein Stück 
weiterzukommen.

England, nicht usa. :-)

[Peter S. (petershaw)]

Hallo Karl Heinz,

das klingt logisch. Ich muss sofort nach Hause und das ausprobieren.
Vielen Dank für den Anstoß!

[Karl H. (kbuchegg)]

Peter Shaw schrieb:
> Sorry wegen der vielen Files. Im XCode ist das schön übersichtlich.
> Jedenfalls für meine Arbeitsweise.
>
> Ich hab in meinem Testcode die Submenüs auch mal rausgenommen.
>
> Das mit den Casts nehme ich mit zu herzen - da wo ich herkomme benutzt
> man diese (viid *) zur Leserlichkeit (um im Aufrug zu sehen was unten
> Sache ist). Also wenn ich eine void *fn_hello(void) habe und diese im
> Aufruf einer Funktion mitgebe hatte ich mal gelernt aus diesem grung
> nicht call(fn_hello()) anzugeben, sondern hier den Wert zu "versichern"
> call((void *)fn_hello)

Sorry. Aber das ist kompletter Blödsinn.

Was du machst ist, du legst absichtlich den Sicherheitsgurt im Auto lahm 
und damit das Piepsen endlich aufhört, steckst du eine Schnalle ins 
Gurtschloss und denkst damit würdest du sicher Autofahren, weil ja das 
Auto nicht mehr piepst.

Sache wäre es, wenn die Menu-Add Funktion einen Funktionbspointer haben 
will. Und zwar nicht einfach irgendeinenen, sondern von genau dem Typ, 
den sie braucht. Und Sache ist es, wenn dir der COMpiler auf die Finger 
klopft wenn dem nicht so ist.

Was hindert dich daran, sowas zu schreiben?

    double kl = 8.6;

    ui_menu_add((char *)&kl, (void *)5 );

richtig. Niemand hindert dich daran. Durch deine Casts hast du nur eines 
erreicht: Du hast deinem Compiler gesagt "Shut up. I am the boss and 
what I do is correct" But the point is: it is not - you just silenced 
your compiler which correctly would have given an error if you had 
omitted the casts and thus would have saved your live, if you just let 
him do his work.


> sondern hier den Wert zu "versichern"
> call((void *)fn_hello)

Das ist KEINE Versicherung. This is NOT an insurance.
THis is your way of saying: I am the boss, you better do what I tell you 
to do. And if I tell you nonsense, then you better do nonsense. It is 
the complete opposite of insurance.

[Thomas E. (thomase)]

Karl Heinz Buchegger schrieb:
> dann wird da alles mögliche gemacht, aber von den Pointern werden gerade
> mal 2 intialisiert.
Das hat er auch in seinem Film so vorgestellt. Der erste und letzte.

Aber hier(main.c) werden insgesamt 5 angelegt:

ui_menu_add((char *)("Say Hello"), (void *)fn_sayHello );
ui_menu_add((char *)("Count ext ports"), (void *fn_countExtPorts );
ui_menu_add((char *)("Show DAC values"), (void *)fn_showDACValue );    
menuentry_t *optionsMenu = ui_menu_add((char *)("Options:\n(Submenu)"), (void *)dummy );
ui_menu_add_sub(optionsMenu, (char *)("Send data via\nUART"), (void *)opt_uartonoff_init );

mfg.

[Karl H. (kbuchegg)]

Thomas Eckmann schrieb:
> Karl Heinz Buchegger schrieb:
>> dann wird da alles mögliche gemacht, aber von den Pointern werden gerade
>> mal 2 intialisiert.
> Das hat er auch in seinem Film so vorgestellt. Der erste und letzte.

Ich red von diesen hier

typedef struct menuentry {
    int position;
    struct menuentry *next;     // pointer to the next entry
    struct menuentry *parent;   // pointer to the parent entry
    struct menuentry *submenu;  // submenu of this item
    char *data;                 // the raw data string to show
    void (*fn)();               // function to execute at startup
} menuentry_t;

[Karl H. (kbuchegg)]

Karl Heinz Buchegger schrieb:

> Ich red von diesen hier
>
>

> typedef struct menuentry {
>     int position;
>     struct menuentry *next;     // pointer to the next entry
>     struct menuentry *parent;   // pointer to the parent entry
>     struct menuentry *submenu;  // submenu of this item
>     char *data;                 // the raw data string to show
>     void (*fn)();               // function to execute at startup
> } menuentry_t;
>

Insbesondere das

menuentry_t *ui_menu_add_sub(menuentry_t *parent, char* data, void* fn){
    menuentry_t *this;
    menuentry_t *last;

    this = (menuentry_t *)parent->submenu;
    last = this;
    
    while(this != NULL){
        last = this;
        this = this->submenu;
    }

...

wird ordentlich in die Hose gehen, wenn nicht garantiert ist, dass 
submenu bei einem neuen Menü Eintrag auch NULL ist. Ist es das?

menuentry_t *ui_menu_add(char* data, void* fn){
    // get the last entry number
    menuentry_t *last = menu->lastentry;
    int lastposition = last->position;

    if( lastposition > -1){
        // if this should be a sub item
        // create a new item
        menuentry_t *entry = malloc(sizeof (menuentry_t));
        
        entry->position = ++lastposition;
        entry->data = data;
        entry->fn = fn;
        entry->next = menu->firstentry;
        
        last->next = entry;
        menu->lastentry = entry;
        
        return entry;

Nope - ist es nicht.

[Peter S. (petershaw)]

Was mir auch gerade auffällt. Ich habe gar keine \0 Terminirung in 
meinem Strings. Kann das auch zu diesem Problem führen?

[Karl H. (kbuchegg)]

Peter Shaw schrieb:
> Was mir auch gerade auffällt. Ich habe gar keine \0 Terminirung in
> meinem Strings. Kann das auch zu diesem Problem führen?

Doch hast du.

"NO DATA"

ist ein 0-terminierter String. Alle String-Literale sind automatisch 
0-terminiert.

Kann es sein, dass du ein C-Buch brauchst?
Wenn du Engländer bist, dann lege ich dir die Orignalausgabe von 
Kernighan&Ritchie sehr ans Herz.
K&R - The C Programming Language

[Thomas E. (thomase)]

Karl Heinz Buchegger schrieb:
> Ich red von diesen hier
Ok.

mfg.

[Peter S. (petershaw)]

PROBLEM SOLVED!
Danke, der hängende Pointer war es. Setzen auf NULL hilft.

Vielen lieben Dank für den super Input.

(So und gleich mal nach den Tipps refactoren)