Rückgabepointer in Funktionsaufruf

[Hundertvolt (Gast)]

Hallo, sorry für die Anfängerfrage, aber ich hab nix dazu finden können.

Ich möchte einer Funktion im Aufruf einen Pointer übergeben, an dessen 
Ziel sie dann Werte schreibt und die dann in der aufrufenden Funktion 
(und nur dort) verfügbar sind. Das ganze hab ich hier in den Tutorials 
schon mit Arrays gesehen - ich möchte das aber mit einem short int 
machen. Hab mir bei erfahrenen Programmierern im Bekanntenkreis schon 
Tips geholt und bin darauf gekommen (vereinfacht):

void Funktion(unsigned short int *Wert)
{
 *Wert = 12345;
}


void Aufruf(void) 
{
 char output[8];
 unsigned short int value;
  
  Funktion(&value); 
  
  utoa(value, output, 10);
  uart_puts(output);
  uart_puts("\n\r");
}

So, und das klappt manchmal, besonders dann, wenn in der "richtigen" 
Funktion wenige Variablen benutzt werden, manchmal kommen völlig 
zufällige Werte raus. Das schreit doch danach, dass manchmal ein 
Zufallstreffer auf den Stack hinhaut, manchmal nicht.

Ich hab scheinbar einige Fakten zur Gültigkeit von Variablen noch nicht 
ganz verstanden, nur finde ich dazu nix brauchbares. Wie bring ich dem 
Compiler bei, was ich will??

[Johann L. (gjlayde)]

Das sieht soweit ok aus.

Johann

[Hundertvolt (Gast)]

Gut, so viel zur Theorie, das freut mich erstmal. Dann mal die ganze 
Funktion - die Zuweisung für den Pointer erfolgt unten, im Case 3:

static unsigned char PSread;                  // Flag: Start Sensorwert auslesen

unsigned char P_Sens_Read(unsigned short int *Wert)
{
 unsigned static char state = 0;                // Zustandsvariable
 char receive[4];                        // Empfangspuffer
 unsigned char Temp;
 
 switch (state)
  {
  case 0:                            // idle
    if (PSread != 0)                    // Startsignal gesetzt?
      {
      if (FastTimerBusy == 0)                // ist der Timer verfügbar?
        {
        Temp = SREG;
        cli();
        FastTimerSet = 6;                // ohne Interrupt Timer auf ca. 1ms stellen
        SREG = Temp;
        PORTB |= (1<<PB4);                // Sensor einschalten
        FastTimerBusy = 1;                // Timer belegen (1 = Drucksensor)
        state = 1;                    // Wechsel in nächsten Zustand
        }
      return 1;                      // Rückgabe: gestartet, warten...
      }
    return 2;                        // Rückgabe: nicht gestartet weil nicht angefordert
    
  case 1:                            // Warten auf Timer-Ablauf
    Temp = SREG;
    cli();
    if (FastTimerSet == 0)                  // ist der Timer abgelaufen?
      {
      state = 2;                      // Zeit abgelaufen, in nächsten Zustand wechseln
      FastTimerBusy = 0;                  // Timer wieder freigeben
      }
    SREG = Temp;
    return 1;                        // Rückgabe: noch warten...
  
  case 2:                            // warten auf Verfügbarkeit SPI
    if (spi_puts("111", PA7) == 0)              // beliebigen Wert schicken (ADC sendet nur, empfängt nicht)
      {
      state = 3;                      // wenn erfolgreich, Zustand wechseln
      }
    return 1;                        // Rückgabe: noch warten...
    
  case 3:                            // Warten auf Sensorwert über SPI
    if (spi_gets(receive) == 0)              // Empfang fertig?
      {
      
      
      //*Wert = ((receive[0] << 12) + (receive[1] << 4) + (receive[2]  >> 4));  // ausgelesenen Wert passend machen
      *Wert = 12345;
      
      
      PORTB &= ~(1<<PB4);                // Sensor ausschalten
      PSread = 0;                      // Startsignal löschen
      state = 0;                      // zurück in Idle
      return 0;                      // Rückgabe: Wert ist gültig
      }
    return 1;                        // sonst: Rückgabe: noch warten...
  }
  return 2;                          // Abschluss (wird nie erreicht)
}

und der Funktionsaufruf:

void P_Sens_Eval(void) 
{
 char output[8];
 unsigned short int value;
  
 if(P_Sens_Read(&value) == 0) 
  {
  utoa(value, output, 10);
  uart_puts(output);
  uart_puts("\n\r");
  }
}

damit lese ich über die SPI einen Sensor aus. Was die restlichen 
Funktionen machen ist prinzipiell egal, das funktioniert. Ich habe die 
Stelle, an der der Wert zurückgegeben wird, etwas freigestellt. Ich 
dachte erst, meine Berechnung würde vielleicht Mist machen und hab sie 
auskommentiert und durch eine Konstante ersetzt.

Nur schon mal so viel: wenn ich statt der Pointerübergabe an dieser 
Stelle die utoa und Uart-Ausgabe mache, kommt der richtige Wert bei 
raus. Bei einer Pointerübergabe kommt nur Müll rüber, der mitunter sogar 
zum Absturz führt. Ich werte die Pointer-Rückgabe - wie zu sehen - nur 
dann aus, wenn der Return-Wert der Funktion auch dessen Gültigkeit 
anzeigt.

Noch was: Wenn ich probehalber die Pointerzuweisung außerhalb der 
Switch-Case-Anweisung mache, kommt meistens das richtige Ergebnis an, 
innerhalb der Switch-Case kommt - egal wo - grundsätzlich immer Mist 
raus.

[Johann L. (gjlayde)]

Hier gibt es nun wirklich zu viel undefiniertes Zeug, über das man 
bestenfalls spekulieren kann. Denkbar sind

-- Pufferüberlauf von receive[]
-- Stacküberlauf
-- Probleme mit dem asynchronen Datenaustausch irgendwelcher ISRs

Es ist Rumgerate mit so wenig Info. Wieso muss der Zugriff auf FastTimer 
einmal atomar sein, das andere mal nicht? Wieviel RAM ist zur Laufzeit 
noch frei? etc etc etc...

Offenbar macht irgendwas den Stack kaputt. Das kann überall sein, auch 
in deinem "funktionierenden" Teil. *Wert bewirkt dann, daß mehr Stack 
gebraucht wird, und es crasht.

Johann

[Alemagnia (Gast)]

>>>char output[8];  <----> utoa(value, output, 10);

Was soll des? Spare?

char o[10];

utoa(v,o,9);

Latch des Baite isch nämmlich Null!

[Stefan E. (sternst)]

Alemagnia schrieb:
>>>>char output[8];  <----> utoa(value, output, 10);
>
> Was soll des? Spare?
>
> char o[10];
>
> utoa(v,o,9);
>
> Latch des Baite isch nämmlich Null!

Unsinn.
Der letzte Parameter ist die Zahlenbasis, nicht etwa die Länge.

[Alemagnia (Gast)]

Also gut.


in Case 3

*Wert = 12345;

Dieser Wert wird lokal im Block der mit dem Compound Statement 
"{"eingeleitet wird angelegt und mit der Beendigung des Blocks "}" 
wieder freigegeben. Nach der Rückkehr aus der Funktion ist dieser Wert 
nicht mehr gültig.

Du kannst es als static deklarieren, dann könnte es funktionieren.

also: static int * Wert;

Leider schlägt der Wert dem globalen Speicherbereich zu Buche und ist 
damit verloren. Besser ausserhalb der Funktion deklarieren.

Viel Glück

[Johann L. (gjlayde)]

Alemagnia schrieb:
> Also gut.
>
>
> in Case 3
>
> *Wert = 12345;
>
> Dieser Wert wird lokal im Block der mit dem Compound Statement
> "{"eingeleitet wird angelegt und mit der Beendigung des Blocks "}"
> wieder freigegeben. Nach der Rückkehr aus der Funktion ist dieser Wert
> nicht mehr gültig.

Nö. Die Adresse von value ist gültig bis zum Verlassen von P_Sens_Eval.
> Du kannst es als static deklarieren, dann könnte es funktionieren.
>
> also: static int * Wert;

Sicher nicht in einer Parameterliste...

> Viel Glück

value static zu machen ist hingegen durchaus sinnig. Wenn's dann 
funktioniert, dann aber möglicherweise nur deshalb, weil weniger Stack 
belegt wird. Früher oder später rasselt's dann woanners...

Johann

[Alemagnia (Gast)]

Zwischenzeitlich hat sich übrigens auch die Source verändert, man 
betrachte auch den Whitespace nach case 3:

Na sowas

[Alemagnia (Gast)]

Aber zugegeben 4 Byte Empfangspuffer hört sich wenig an. Und dann cli() 
ohne ein sti(). Aber wurde ja schon erwähnt...

[Hundertvolt (Gast)]

> Dieser Wert wird lokal im Block der mit dem Compound Statement
>"{"eingeleitet wird angelegt und mit der Beendigung des Blocks "}"
>wieder freigegeben. Nach der Rückkehr aus der Funktion ist dieser Wert
>nicht mehr gültig.


das dachte ich doch, hätte ich gerade dadurch erledigt, dass ich doch 
die Variable in der aufrufenden Funktion (P_Sens_Eval) deklariere und 
dann an P_Sens_Read nur einen Pointer darauf übergebe. Und ist es nicht 
so, dass die Variable innerhalb des Aufrufs von P_Sens_Eval gültig ist 
und damit auch dann, wenn P_Sens_Eval die Funktion P_Sens_Read aufruft? 
So wurde es mir jedenfalls erklärt. Und damit sollte ich die Variable 
innerhalb von P_Sens_Read benutzen können.

Einen Stacküberlauf kann ich mir nicht vorstellen. Ich hab diese Art an 
Pointerübergabe auch so ähnlich - und (zufällig???) erfolgreich - an 
anderen Funktionen eingesetzt, die viel mehr Variablen haben und auch 
richtig Text an die UART (mit Ringpuffer) schreiben. Den Ringpuffer 
hatte ich da auf der Länge 90 (!) für RX und TX - und vollgeschrieben. 
Kein Ärger. Jetzt steht er auf 25, die ganzen anderen Funktionen sind 
stillgelegt, aber es kracht. SPI_Receive läuft auch nicht über. Dann 
könnte ich ja nicht bei Verzicht auf Pointer den richtigen Wert 
ausgeben.

Zur weiteren Info: FastTimerBusy ist nur ein Infoflag, ob und welche 
Funktion den Timer belegt und wird niemals in ISRs zugegriffen. 
FastTimerSet ist eine Countdown-Variable, die in der ISR zum Compare 
Match bis 0 runtergezählt wird. Daher nichtatomarer / atomarer Zugriff.

als static oder volatile deklarieren bringt nichts. Ich will auf jeden 
Fall um eine permanente Speicherbelegung herumkommen, sonst hätte ich 
das ja mit einer globalen Variablen lösen können.

An der Source hab ich nichts verändert, das muss ne optische Täuschung 
sein. Die SPI hat aktuell 20 Byte RX / TX Puffer. Aber der ADC, den ich 
auslese, spuckt ohne weitere Befehle immer genau 24 bit = 3 Byte + 1 
Byte für \n als Stringende. Also nicht knapp, sondern genau richtig.

Ach ja, und cli() ohne sti() und dafür der Trick mit SREG - das findet 
ich als "guter Programmierstil" im Tut... :-)

[Johann L. (gjlayde)]

Hundertvolt schrieb:
> Aber der ADC, den ich
> auslese, spuckt ohne weitere Befehle immer genau 24 bit = 3 Byte + 1
> Byte für \n als Stringende. Also nicht knapp, sondern genau richtig.

Stringende in C ist ein \0.

Johann

[Alemagnia (Gast)]

Du ich weiss es wirklich nicht. Wie auch meine Vorredner finde ich den 
Code soweit korrekt aber nicht vollständig genug um es beurteilen zu 
können.

Du musst einfach mehr künstliche Breakpoints mit Kontrollausgaben 
reinsetzen oder Schritt für Schritt mit dem Debugger durch. Geht 
natürlich nicht wenn Interrupts dazwischenfunken die muss man dann auch 
noch simulieren.

Als Programmieranfänger macht man immer zu viele Schritte auf einmal 
anstatt Schritt für Schritt. Wenn man fremden Code einsetzt ist es auch 
immer ein wenig ein Glückspiel. Noch schlimmer wird es wenn man fremden 
Code und eigenen mixt.

[Hundertvolt (Gast)]

>Stringende in C ist ein \0.

ich weiß. Sorry, Tippfehler. ;-)

Ich hab das mit Debugausgaben schon gemacht - funzt wunderbar. Halt bis 
zu der Stelle der Pointerübergabe.

Ich find besonders komisch, dass es immer genau dann kracht, wenn die 
Pointerübergabe innerhalb der Switch-Case passiert, egal an welcher 
Stelle, außerhalb geht es ohne weitere Veränderung des Codes perfekt.

>Als Programmieranfänger macht man immer zu viele Schritte auf einmal
>anstatt Schritt für Schritt. Wenn man fremden Code einsetzt ist es auch
>immer ein wenig ein Glückspiel. Noch schlimmer wird es wenn man fremden
>Code und eigenen mixt.

soo ein Anfänger bin ich gar nicht, nur ein wenig aus der Übung ^^
Den Code hab ich komplett selber geschrieben, und bis dato habe ich 7 
parallele Interrupts (diverse Timer, UART, SPI, extern,...) ohne jede 
Macke am Laufen. Und jetzt das.... nerv ;-)

[Klaus W. (mfgkw)]

die 7 helfen dir aber nichts, wenn du sie mit cli() abschaltest und nie
wieder ein...

Vielleicht läuft dir dadurch dann ja woanders etwas über, was man an 
deinem Ausschnitt hier nicht sieht?

[Patrick D. (oldbug)]

An alle, die sich über das cli() wundern: denkt doch mal darüber nach:

  /* ... */
  Temp = SREG;  /* SREG sichern */
  cli();        /* Global interrupts ausschalten */

  /* Zugriff hier "Atomar" */
  x = y;

  SREG = Temp;  /* SREG wiederherstellen */

Sowas findet man auch schon mal als Makro mit dem Namen "ENTER_CRITICAL" 
o.ä.

[Klaus W. (mfgkw)]

ok, Lesen hilft

[Alemagnia (Gast)]

>Ich hab das mit Debugausgaben schon gemacht - funzt wunderbar. Halt bis
>zu der Stelle der Pointerübergabe.

Die Pointerübergabe ist aber prinzipiell korrekt so.

Wenn es da abstürzt hat sich der Pointer "Wert" möglicherweise 
verändert.

Wenn du eine Debugausgabe von Wert als Pointervalue machst, dann müsste 
es klar werden.

http://www.mikrocontroller.net/articles/Interrupt#Wichtige_Eigenschaften_von_ISRs

Denkbar wäre zum Beispiel ein verschachtelter Interrupt, sowas darf man 
nicht zulassen.