NULL Pointer als Übergabe in Funktion verursacht sporadische Fehler

Gast #4837163
Hallo,
wenn ich folgende Funktion des öfteren aufrufe, kommt es zu sporadischen 
Fehlern im Programmablauf. Der Speicher verändert sich an gewissen 
Stellen.
Es liegt an der Übergabe des NULL-Zeigers, aber warum? Es ist eine 32Bit 
Architektur von Atmel (sam d21)
1
PS_ProgrammEvent(EVENT_PUSHNACHRICHT, NULL, 0, NULL);

Header:
1
void PS_ProgrammEvent(Event_Typ Event, uint8_t *Param, uint16_t len, uint8_t *id);

Funktion:
1
void PS_ProgrammEvent(Event_Typ Event, uint8_t *Param, uint16_t len, uint8_t *id)
2
{
3
  uint8_t LenByte;
4
  Programm_Typ Prog[2];  
5
  bool Setzen[2];          // Standardmäßig löschen der Aktivierung des Programms.
6

7
  memset(&Prog[0], 0, sizeof(Prog[0]));
8
  
9
  // Prüfen auf Verträglichkeit
10
  if(len > sizeof(InfoTable[0].Param))
11
    return;
12

13
  // Prüft dein Event gegen und schaltet die Programme um, falls nötig.
14
  switch(Event)
15
  {  
16
    case EVENT_EINGEHENDER_ANRUF:
17

18
      // Ein Anruf geht gerade ein.
19
      Prog[0] = PROG_EINGEHENDER_ANRUF;
20
      Setzen[0] = 1;    
21
                        
22
    break;
23
    case EVENT_EINGEHENDER_ANRUF_BEENDET:
24

25
      // Eingehender Anruf ist beendet.      
26
      Prog[0] = PROG_EINGEHENDER_ANRUF;              
27
      Setzen[0] = 0;  
28

29
    break;
30
    case EVENT_VERPASSTER_ANRUF:
31
    
32
      Prog[0] = PROG_VERPASSTER_ANRUF;
33
      Setzen[0] = 1;
34
    
35
    break;
36
    case EVENT_VERPASSTER_ANRUF_GELESEN:
37
    
38
      Prog[0] = PROG_VERPASSTER_ANRUF;
39
      Setzen[0] = 0;
40

41
    break;
42
    case EVENT_VERPASSTE_PUSHNACHRICHT:
43
    
44
      Prog[0] = PROG_VERPASSTE_NACHRICHT;
45
      Setzen[0] = 1;
46
    
47
    break;
48
    case EVENT_PUSHNACHRICHT_GELESEN:  
49
      
50
      Prog[0] = PROG_VERPASSTE_NACHRICHT;
51
      Setzen[0] = 0;    
52

53
    break;
54
    case EVENT_PUSHNACHRICHT:  
55
        
56
      Prog[0] = PROG_EINGEHENDE_NACHRICHT;
57

58
    break;
59
    case EVENT_BLE_VERBUNDEN:
60

61
      Prog[0] = PROG_BLE_CONNECTED;
62
      Setzen[0] = 1;
63

64
      Prog[1] = PROG_BLE_DISCONNECTED;
65
      Setzen[1] = 0;
66

67
    break;
68
    case EVENT_BLE_GETRENNT:
69

70
      Prog[0] = PROG_BLE_CONNECTED;
71
      Setzen[0] = 0;
72

73
      Prog[1] = PROG_BLE_DISCONNECTED;
74
      Setzen[1] = 1;
75

76
    break;
77
  }    
78

79
  // Gehe alle Vorhaben durch:
80
  for(uint8_t b = 0; b < sizeof(Prog)/sizeof(Prog[0]); b++)
81
  {  
82
    if(Prog[b] == PROG_UNDEFINIERT)
83
      continue;
84
    
85
    // Gehe alle Programme durch.
86
    for(uint8_t a = 0; a < SYS_AnzahlZustaende; a++)
87
    {
88
      // Gibt es ein Programm, dass aktiviert werden kann?
89
      if(InfoTable[a].ProgTyp == Prog[b])
90
      {
91
        switch(InfoTable[a].ProgTyp)
92
        {
93
          // Programmtyp passt.
94
          case PROG_EINGEHENDER_ANRUF:
95
            // TODO: Prüfe die Parameter des Programms.
96
                    
97
            InfoTable[a].Aktiviert = Setzen[b];  
98
                                        
99
          break;
100
          case PROG_VERPASSTE_NACHRICHT:  
101
                  
102
            // Prüfe den Parameter (App-ID)
103
            if(Setzen[b])
104
            {
105
              // Prüfe den Parameter:
106
              LenByte = InfoTable[a].Param[0];
107

108
              if(!LenByte || LenByte <= PS_ProgrammInfoParaLaenge && len == LenByte && !memcmp(&InfoTable[a].Param[1], Param, LenByte))
109
              {
110
                // Speicher die Paket ID ab:
111
                memcpy(&InfoTable[a].NotiID[0], id, sizeof(InfoTable[a].NotiID));
112
                InfoTable[a].Aktiviert = Setzen[b];                
113
              }                        
114
            }
115
            else if(!Setzen[b] && !memcmp(&InfoTable[a].NotiID[0], id, sizeof(InfoTable[a].NotiID)))
116
            {  
117
              InfoTable[a].Aktiviert = Setzen[b];              
118
            } 
119

120
          break;
121
          case PROG_VERPASSTER_ANRUF:
122
            // Verpasster Anruf:
123
            if(Setzen[b])
124
            {
125
              // Länge des Namens:
126
              LenByte = InfoTable[a].Param[0];
127

128
              // Wenn die Länge des hinterlegten Namens kleiner ist als die des Parameters.
129
              if(LenByte < len)              
130
                len = LenByte;              
131

132
              if(!LenByte || LenByte <= (PS_ProgrammInfoParaLaenge - 1) && len == LenByte && !memcmp(&InfoTable[a].Param[1], Param, LenByte))
133
              {
134
                // Speicher die Paket ID ab:
135
                memcpy(&InfoTable[a].NotiID[0], id, sizeof(InfoTable[a].NotiID));
136
                InfoTable[a].Aktiviert = Setzen[b];
137
              }
138
            }
139
            else if(!Setzen[b] && !memcmp(&InfoTable[a].NotiID[0], id, sizeof(InfoTable[a].NotiID)))
140
            {
141
              InfoTable[a].Aktiviert = Setzen[b];
142
            }
143
          break;
144
          case PROG_EINGEHENDE_NACHRICHT:
145

146
            // Übergangsprogramm einer eingehenden Nachricht:
147
            InfoTable[a].Aktiviert = 1;
148

149
          break;
150
          case PROG_BLE_CONNECTED:
151

152
            InfoTable[a].Aktiviert = Setzen[b];
153

154
          break;
155
          case PROG_BLE_DISCONNECTED:
156

157
            InfoTable[a].Aktiviert = Setzen[b];
158

159
          break;
160
        }
161
      }
162
    }
163
  }
164
  
165
  PS_ProgrammeUebernehmen();
166
}
#4837172
Also du iterierst über a und b, und nimmst die als Index für "Setzen", 
welches immer nur die Länge 2 hat. Dann liest du definitiv durchaus mal 
zufällig kram vom Stack/Speicher.
Das selbe passiert mit "id" und "Param", werden gelesen, obwohl sie ggf. 
NULL sind.
Der Code hat mir zu wenig defensive Checks eingebaut.
Evtl. würde es auch helfen, wenn du die Routine etwas aufspaltest in 
verschiedene Funktionen - für die Verständlichkeit, wobei die an sich
gut ist mit den Variablennamen und Kommentaren. Auch wenn mir noch
zuviele Unbekannte da sind, bspw. wie groß InfoTable wirklich ist,
und warum es abhängig ist von SYS_AnzahlZustaende, müsste sie dann nicht
StateTable oder ZustandsTable heißen?
Gast #4837175
der code ist nicht gerade gut zu lesen, viel zu verschachtelt.

im ersten Moment kann ich aber keinen Fehler finden, kann es sein das es 
in  PS_ProgrammeUebernehmen() zu einen Fehler kommt.
#4837181
Was mir auch noch auffällt:
1
if(Prog[b] == PROG_UNDEFINIERT)

Du setzt nirgends PROG_UNDEFINIERT. Wozu ist dann
der Check da? Du kannst nicht annehmen, dass deine
Strukturen und Variablen auf dem Stack mit 0 initialisiert
sind! Du setzt ja nur Prog[0] auf 0 mit:
1
memset(&Prog[0], 0, sizeof(Prog[0]));
Gast #4837184
Robin R. schrieb:
> Das selbe passiert mit "id" und "Param", werden gelesen, obwohl sie ggf.
> NULL sind.

Nö werden sie von dem Funktionsaufruf nicht. Wie willst du einen Pointer 
denn auf Länge prüfen?

Rufus Τ. F. schrieb:
> Kann nicht sein. Die Funktion wertet ihren zweiten Parameter nirgends
> aus.

Eben aber wenn ich den Funktionsaufruf noppe funktioniert alles Stabil.
Nur dieser Aufruf verursacht den Fehler nach einigen Aufrufen.

Peter II schrieb:
> im ersten Moment kann ich aber keinen Fehler finden, kann es sein das es
> in  PS_ProgrammeUebernehmen() zu einen Fehler kommt.

Passiert nichts dolles:
1
void PS_ProgrammeUebernehmen(void)
2
{
3
  // Übernimmt das aktivierte Programm mit der höchsten Priorität:
4
  bool KeinProgrammGefunden = 1;
5

6
  // Prüfe auf Umschalten - Grundprogramm wird ignoriert:
7
  for(uint8_t a = 1; a < SYS_AnzahlZustaende; a++)
8
  {
9
    if(InfoTable[a].Aktiviert)
10
    {
11
      if(LS_AktuellesProgramm != a)
12
      {
13
        LS_AktuellesProgramm = a;
14
        LS_ProgrammNeuLaden = 1;
15
      }
16
      
17
      KeinProgrammGefunden = 0;
18
      break;
19
    }
20
  }
21

22
  // Wenn es keine aktivierten Programme gibt, schalte in das 0. Programm!
23
  if(KeinProgrammGefunden && LS_AktuellesProgramm != 0)
24
  {
25
    LS_AktuellesProgramm = 0;
26
    LS_ProgrammNeuLaden = 1;
27
  }
28
}
Gast #4837185
Robin R. schrieb:
> Du setzt nirgends PROG_UNDEFINIERT.

Doch:
1
typedef enum{
2
  EVENT_EINGEHENDER_ANRUF,
3
  EVENT_EINGEHENDER_ANRUF_BEENDET,
4

5
  EVENT_VERPASSTER_ANRUF,
6
  EVENT_VERPASSTER_ANRUF_GELESEN,
7

8
  EVENT_VERPASSTE_PUSHNACHRICHT,
9
  EVENT_PUSHNACHRICHT_GELESEN,
10

11
  EVENT_BLE_VERBUNDEN,
12
  EVENT_BLE_GETRENNT,
13

14
  // Blending Programme (Übergangsprogramme):
15
  EVENT_PUSHNACHRICHT,
16
} Event_Typ;
Gast #4837187
@ Ralf

Andere hier reagieren vielleicht anders und mehr, wie Du es wünschst, 
aber ich muss leider sagen, dass ich mir diesen Code nicht antue.

Dafür gibt es abstrakte Gründe, die Dir vielleicht nützlich erscheinen, 
vielleicht aber auch nicht:

Zum einen hast Du sehr viel Code in eine Funktion geklemmt. Das ist 
nicht eigentlich falsch, führt aber dazu, dass sie nur mit hohem Aufwand 
zu testen ist. Ich rate Dir, die Funktion in mehrere kleinere 
aufzuteilen. Eine bewährte Daumenregel lautet: "Eine Aufgabe, eine 
Funktion" und eine weitere, ebenso bewährte: "Eine Funktion, eine 
Bildschirmseite" (also etwa 25 Zeilen à 80 Zeichen).

Zum anderen ist die Fehlerbeschreibung bzw. die Analyse zu 
oberflächlich.
Es fängt damit an, dass eine NULL als Parameter an sich niemals einen 
Fehler hervorruft, wenn man voraussetzt, dass als Parameter an sich ein 
Zeiger zulässig ist. Das wusstest Du vielleicht nicht.
Aber wenn man das voraussetzt, dann ruft eine NULL effektiv einen Fehler 
bei ihrer Verwendung in einem Ausdruck einen Fehler hervor. Die Stelle 
an der das geschieht musst Du finden und mit einer Wahrscheinlichkeit 
ist auch ziemlich offensichtlich, was da genau schiefläuft - leider 
nicht immer, oft eben auch nicht.

Also bitte überarbeite Deinen Code. Überlege Dir Tests für die 
Einzelfunktion und baue die dann in die Funktion PS_ProgrammEvent ein.
Gast #4837192
Ralf schrieb:
> Rufus Τ. F. schrieb:
>> Kann nicht sein. Die Funktion wertet ihren zweiten Parameter nirgends
>> aus.
>
> Eben ...

Das sehe ich anders:

In dieser Zeile:
1
if(!LenByte || LenByte <= PS_ProgrammInfoParaLaenge && len == LenByte && !memcmp(&InfoTable[a].Param[1], Param, LenByte))

und in dieser Zeile
1
if(!LenByte || LenByte <= (PS_ProgrammInfoParaLaenge - 1) && len == LenByte && !memcmp(&InfoTable[a].Param[1], Param, LenByte))

wird Param verwendet.
Gast #4837193
Ralf schrieb:
> Du setzt nirgends PROG_UNDEFINIERT.

Sry. Meinte das hier:
1
typedef enum{
2
  // Stabile Programme (Hauptprogramme)
3
  PROG_UNDEFINIERT = 0,                            // Undefiniertes Programm
4
  PROG_VERPASSTER_ANRUF = 1,                          // Nicht angenommener Anruf
5
  PROG_VERPASSTE_NACHRICHT = 2,                        // Ungelesene Nachricht
6
  PROG_EINGEHENDER_ANRUF = 3,                          // Eingehender Anruf
7
  PROG_BLE_CONNECTED = 4,                            // iPhone per Bluetooth verbunden
8
  PROG_BLE_DISCONNECTED = 5,
9

10
  // Blending Programme (Übergangsprogramme)
11
  PROG_EINGEHENDE_NACHRICHT = 254,
12
} Programm_Typ;
#4837194
Ralf schrieb:
> Robin R. schrieb:
>> Das selbe passiert mit "id" und "Param", werden gelesen, obwohl sie ggf.
>> NULL sind.
>
> Nö werden sie von dem Funktionsaufruf nicht. Wie willst du einen Pointer
> denn auf Länge prüfen?

Was und ob du auf id oder Param zugreifst hängt allein vom Inhalt
in InfoTable[a] ab. Außerdem verstehe ich nicht was du mit Länge meinst.
Ein NULL-Pointer zeigt auf 0x0, und Lesezugriffe davon sind schonmal
bedenklich.

Ehrlich, bau mehr Sicherheitsprüfungen ein und initialisiere deine
Variablen richtig. Dann schau ob die Fehler weg gehen.
Gast #4837203
Eine zweifach verschachtelte Schleife mit einer mehrdimensionalen 
Statemachine in der inneren Schleife? WTF? Soll das später noch 
irgendwer anderes warten, oder ist das bloß Wegwerfcode zum privaten 
Eigengebrauch?
Gast #4837205
Nop schrieb:
> Eine zweifach verschachtelte Schleife mit einer mehrdimensionalen
> Statemachine in der inneren Schleife? WTF? Soll das später noch
> irgendwer anderes warten, oder ist das bloß Wegwerfcode zum privaten
> Eigengebrauch?

Nö, für die Tonne.
Gast #4837207
Ralf schrieb:
> Theor schrieb:
>> wird Param verwendet.
>
> Ja aber nicht bei dem konkreten Aufruf.

Richtig. Wenn Du den oben zitierten Aufruf machst, dann nicht.
Dann wird aber auch nichts weiter in der ganzen Funktion getan, als
1
InfoTable[a].Aktiviert = 1;

was in der später geposteten Funktion PS_ProgrammeUebernehmen zunächst 
zu nichts weiter als setzen von zur Compile-Zeit definierten Variablen 
bewirkt.

Darin liegt der Fehler jedenfalls nicht.


Ich verweise nochmal auf meinen Beitrag hier: 
Beitrag "Re: NULL Pointer als Übergabe in Funktion verursacht sporadische Fehler"
#4837208
Ralf schrieb:
> void PS_ProgrammEvent(Event_Typ Event, uint8_t *Param, uint16_t len,
...
>   if(len > sizeof(InfoTable[0].Param))

Du hast also einen Bytepointer *Param und eine Struct mit einem Member 
.Param.
Ist zwar erlaubt, sollte man aber nicht tun, da es sehr verwirrend ist.
Nimm für verschiedene Sachen auch verschiedene Namen.

Wenn Du explizit für Parameter NULL zuläßt, solltest Du sie auch 
explizit auf NULL testen.

Man kann überlange Zeilen auch sinnvoll umbrechen, C ist nicht 
zeilensensitiv.

Das ganze sieht wahnsinnig verworren aus, erweckt aber den Eindruck, daß 
man es viel einfacher und klarer schreiben könnte.
Gast #4837211
Peter D. schrieb:
> Du hast also einen Bytepointer *Param und eine Struct mit einem Member
> .Param.
> Ist zwar erlaubt, sollte man aber nicht tun, da es sehr verwirrend ist.
> Nimm für verschiedene Sachen auch verschiedene Namen.

Guter Anreiz, werde ich ändern!

Peter D. schrieb:
> Das ganze sieht wahnsinnig verworren aus, erweckt aber den Eindruck, daß
> man es viel einfacher und klarer schreiben könnte.

Ist es auch, ist eben eine Abarbeitung von Zuständen. Wenn ich in der 
switch alles in Funktionen Packe, trägt das der Übersichtlichkeit bei, 
das stimmt schon, mache ich ggf. auch.

Grade probiert:
1
PS_ProgrammEvent2(EVENT_PUSHNACHRICHT, NULL, 0, NULL);
1
void PS_ProgrammEvent2(Event_Typ Event, uint8_t *Param, uint16_t len, uint8_t *id)
2
{
3

4
}

[c]void PS_ProgrammEvent2(Event_Typ Event, uint8_t *Param, uint16_t len, 
uint8_t *id);[c]

Verursacht ähnliche Probleme.
Gast #4837214
Ralf schrieb:
> Grade probiert:
>
>
1
PS_ProgrammEvent2(EVENT_PUSHNACHRICHT, NULL, 0, NULL);
>
>
1
void PS_ProgrammEvent2(Event_Typ Event, uint8_t *Param, uint16_t len, 
2
> uint8_t *id)
3
> {
4
> 
5
> }
>
> [c]void PS_ProgrammEvent2(Event_Typ Event, uint8_t *Param, uint16_t len,
> uint8_t *id);[c]
>
> Verursacht ähnliche Probleme.

Schön. Aber was sollen wir nun damit anfangen?
Du musst das Problem erstmal genau feststellen, beschreiben worin es 
besteht. Ich wiederhole: Eine Null als Parameter verursacht an sich 
keinen Fehler.
Gast #4837234
Ralf schrieb:
> Erster produktiver Beitrag, habe ich grade auch bemerkt.
> Genau dadurch entstand der Fehler !

Danke, genauauf die Stelle hab ich schon vor dutzenden Posts 
hingewiesen.
Liest denn keiner meine Posts oder muss man jetzt alles vorkauen?
Gast #4837237
Robin.R. schrieb:
> Ralf schrieb:
>> Erster produktiver Beitrag, habe ich grade auch bemerkt.
>> Genau dadurch entstand der Fehler !
>
> Danke, genauauf die Stelle hab ich schon vor dutzenden Posts
> hingewiesen.
> Liest denn keiner meine Posts oder muss man jetzt alles vorkauen?

Tröste Dich. Da bist Du nicht der einzige hier.
Gast #4837245
Robin.R. schrieb:
> Ralf schrieb:
> Erster produktiver Beitrag, habe ich grade auch bemerkt.
> Genau dadurch entstand der Fehler !
>
> Danke, genauauf die Stelle hab ich schon vor dutzenden Posts
> hingewiesen.
> Liest denn keiner meine Posts oder muss man jetzt alles vorkauen?

Stimmt, hatte ich überlesen, tut mir leid.
Gast #4837252
Können auch gerne mal über die Struktur diskutieren. Die Funktion 
verknüpft Events mit Zuständen, klar kann ich in der switch die Code 
Teile auslagern. Nur die erste switch Anweisung legt quasi die Zustände 
fest, die durch ein Event geändert werden müssen und Prog ist die 
"Warteschlange"

Antwort schreiben

Eine eigene Frage? Dieser Thread ist über 9 Jahre alt. Stell sie in einem eigenen Thread – dort lesen mehr Leute mit, und sie wird öfter und schneller beantwortet.

Neuen Thread starten

Eine Lösung, Ergänzung oder Rückmeldung zu diesem Thread? Dann antworte hier.

Formatierung
  • [c]C-Code[/c]
  • [code]Code in anderen Sprachen, ASCII-Zeichnungen[/code]
  • [math]Formel in LaTeX-Syntax[/math]
  • [[Titel]] – Link zu einem Artikel
  • Verweis auf einen anderen Beitrag: Rechtsklick auf dessen Beitrags-ID (#8078628), „Adresse kopieren“, in den Text einfügen

Mehr zur Formatierung

Längeren Sourcecode als Anhang, nicht in den Text. JPG nur für Fotos und Scans; Zeichnungen und Screenshots als PNG oder GIF. Siehe Bildformate.

Schon registriert? Anmelden

Wird nicht angezeigt. Wir schicken dir einen Link; sobald du damit bestätigst, erscheint dein Beitrag.

Mit dem Abschicken legst du einen kostenlosen Account an und erkennst die Nutzungsbedingungen und die Datenschutzerklärung an.