Forum: Mikrocontroller und Digitale Elektronik Bytes einsammeln, Überlauf Problem?


Announcement: there is an English version of this forum on EmbDev.net. Posts you create there will be displayed on Mikrocontroller.net and EmbDev.net.
von Hansi (Gast)


Bewertung
0 lesenswert
nicht lesenswert
Nabend,

irgendwie habe ich gerade schwerwiegende Denkprobleme ( sollte mal 
lieber Feierabend machen.. )

Kann ich hier einen Speicherüberlauf produzieren, wenn mein Speicher 
"inStream" 1024 Bytes groß ist?
Oder habe ich das korrekt abgefangen?
1
uint8_t *usartReadRingBuff      ( uint8_t *stream )    
2
{
3
  static uint16_t index = 0;
4
  
5
  uint16_t c = usartGetChar();
6
  
7
  if ( c & USART_NO_DATA )
8
  {
9
    return NULL;
10
  }
11
  
12
  if ( c > USART_NO_DATA)
13
  {
14
    index = 0; 
15
    return NULL;
16
  }
17
18
  static uint8_t last = 0;
19
   
20
  if ( last == '\r' && c == '\n' )
21
  {
22
    if ( (index - 1) > sizeof(inStream ) )
23
    {
24
      index = 0;
25
      return NULL;
26
    }
27
    else
28
    {
29
      *( stream + ( index - 1 ) ) = '\0';
30
      index = 0; 
31
    
32
      return stream;      
33
    }
34
  }last = c;  
35
  
36
  if ( index > sizeof(inStream) )
37
  {
38
    index = 0;
39
  }
40
  else
41
  {
42
    *( stream + index++ ) = (uint8_t)c;  
43
  }
44
  
45
  return NULL;
46
}

von guest (Gast)


Bewertung
0 lesenswert
nicht lesenswert
Die Größe von "inStream" ist völlig Banane, Du greifst eh nicht darauf 
zu.
Wesentlich interessante wäre, wo der Funktionsparameter hinzeigt.
Und je nachdem was ein "sizeof(int)" liefert, knallt es dann unter 
Umständen immer noch.

von Stefan ⛄ F. (stefanus)


Bewertung
-1 lesenswert
nicht lesenswert
Die wichtigste Info fehlt: sizeof(inStream)

Das liefert Dir nämlich leider nicht die Größe des zuvor reservierten 
Speichers, sondern die Größe des Zeigers.

von Hmmmm (Gast)


Bewertung
0 lesenswert
nicht lesenswert
Hansi schrieb:
> if ( index > sizeof(inStream) )


index  darf nicht grösser als

(sizeof(inStream)) - 1)

werden.

von Hansi (Gast)


Bewertung
0 lesenswert
nicht lesenswert
Stefanus F. schrieb:
> Die wichtigste Info fehlt: sizeof(inStream)
>
> Das liefert Dir nämlich leider nicht die Größe des zuvor reservierten
> Speichers, sondern die Größe des Zeigers.

Wenn ich auf dem Rechner
1
int main()
2
{
3
  uint8_t buff[] = "Test1234";
4
5
  printf("%d" , sizeof(buff));
6
7
    return 0;
8
}
Gibt er mir exakt die Größe des Array´s aus.

von Stefan ⛄ F. (stefanus)


Bewertung
-2 lesenswert
nicht lesenswert
Hansi schrieb:
> Stefanus F. schrieb:
>> Das liefert Dir nämlich leider nicht die Größe des zuvor reservierten
>> Speichers, sondern die Größe des Zeigers.
>
> Wenn ich auf dem Rechner
>
1
> int main()
2
> {
3
>   uint8_t buff[] = "Test1234";
4
> 
5
>   printf("%d" , sizeof(buff));
6
> 
7
>     return 0;
8
> }
9
>
> Gibt er mir exakt die Größe des Array´s aus.

Ja, weil du dort die Größe des Array abfragst.
Aber in deiner Funktion oben hast du die Größe des Zeigers abgefragt.

von Floppy (Gast)


Bewertung
0 lesenswert
nicht lesenswert
Hansi schrieb:

Deine Funktion hat "RingBuff" im Namen, hat aber mit einem Ringbuffer 
nichts am Hut. Das finde ich jetzt nicht so besonders gut.

Was glaubst du wird passieren, wenn "index == sizeof(inStream)" wird?
>
>   if ( index > sizeof(inStream) )
>   {
>     index = 0;
>   }
>   else
>   {
>     *( stream + index++ ) = (uint8_t)c;
>   }

von Stefan ⛄ F. (stefanus)


Bewertung
0 lesenswert
nicht lesenswert
Du musst deine Funktion so ändern, dass sie zwei Aufrufparameter hat: 
Einen Zeiger auf den Puffer und einen Integer mit dessen Größe.

von Floppy (Gast)


Bewertung
0 lesenswert
nicht lesenswert
@Stefanus
Wo ist oben? Ich sehe nirgends ein sizeof(stream).

von Stefan ⛄ F. (stefanus)


Bewertung
0 lesenswert
nicht lesenswert
@Floppy
Zwei Troll-Versuche genügen. Lass es gut sein und geht woanders hin.

von Hansi (Gast)


Bewertung
0 lesenswert
nicht lesenswert
Stefanus F. schrieb:
> Du musst deine Funktion so ändern, dass sie zwei Aufrufparameter
> hat: Einen Zeiger auf den Puffer und einen Integer mit dessen Größe

Ich glaube ich habe die Bezeichnungen ein wenig unglücklich gewählt..
inStream ist mein Speicher ( inStream[1024]
Und der Funktion übergebe ich den Zeiger auf den Bereich. Also die Größe 
müsste passen?

von Floppy (Gast)


Bewertung
1 lesenswert
nicht lesenswert
Stefanus F. schrieb:
> @Floppy
> Zwei Troll-Versuche genügen. Lass es gut sein und geht woanders hin.

Oh, da ist ja jemand sehr von sich überzeugt!

Stefanus F. schrieb:

> Aber in deiner Funktion oben hast du die Größe des Zeigers abgefragt.

Zeig doch mal, wo das sein soll.

von Stefan ⛄ F. (stefanus)


Bewertung
-2 lesenswert
nicht lesenswert
Wie gesagt liefert "sizeof(inStream)" nicht die von Dir erwarteten 1024, 
sondern je nach Computer wahrscheinlich 2 oder 4.

von Walter S. (avatar)


Bewertung
0 lesenswert
nicht lesenswert
Stefanus F. schrieb:
> Zwei Troll-Versuche genügen. Lass es gut sein und geht woanders hin.

schau dir noch Mal den Parameter von sizeof genau an!

von Hansi (Gast)


Bewertung
0 lesenswert
nicht lesenswert
Stefanus F. schrieb:
> Wie gesagt liefert "sizeof(inStream)" nicht die von Dir erwarteten
> 1024, sondern je nach Computer wahrscheinlich 2 oder 4.

Okay nehmen wir an es würde da jetzt noch die 1024 stehen, würde es denn 
so wie es ist funktionieren ohne Speicher Überlauf?

von Stefan ⛄ F. (stefanus)


Bewertung
0 lesenswert
nicht lesenswert
Hansi schrieb:
> Okay nehmen wir an es würde da jetzt noch die 1024 stehen, würde
> es denn so wie es ist funktionieren ohne Speicher Überlauf?

Es würde nicht funktionieren weil die letzte Index Position die Nummer 
1023 hat (die Zählung beginnt bei 0).

: Bearbeitet durch User
von Hansi (Gast)


Bewertung
0 lesenswert
nicht lesenswert
Okay, also Reservierter_Speicher-1, habe ich das richtig verstanden?

von Stefan ⛄ F. (stefanus)


Bewertung
0 lesenswert
nicht lesenswert
Hansi schrieb:
> Okay, also Reservierter_Speicher-1, habe ich das richtig verstanden?

Du kannst den gesamten reservierten Speicher belegen.

Die erste Speicherzelle ist *( stream + 0 )
Die letzte Speicherzelle ist *( stream + 1023 )

Du kannst übrigens auch "stream[index] = (uint8_t) c;" schreiben, 
bewirkt das Gleiche.

: Bearbeitet durch User
von Hansi (Gast)


Bewertung
-1 lesenswert
nicht lesenswert
Jetzt verstehe ich es nicht mehr.. Wie müsste denn jetzt die if Abfrage 
aussehen? Du verunsicherst mich gerade..

von Experte (Gast)


Bewertung
0 lesenswert
nicht lesenswert
Hallo Hansi,

du hast grundsätzliche Konzepte von C offensichtlich nicht verstanden.
Auch dein Funktionsname usartReadRingBuff zeigt, dass du keinen 
Gendanken an
Softwaredesign/Abstraktion oder Generik/Wiederverwendbarkeit 
verschwendest.

Mein Tip:
Nimm dir eine Standard-Ringbuffer-Implementierung und such dir ein 
Beispiel wo man sieht, wie diese anzuwenden ist.

von Hansi (Gast)


Bewertung
-2 lesenswert
nicht lesenswert
Experte schrieb:
> Hallo Hansi,
>
> du hast grundsätzliche Konzepte von C offensichtlich nicht verstanden.
> Auch dein Funktionsname usartReadRingBuff zeigt, dass du keinen
> Gendanken an Softwaredesign/Abstraktion oder
> Generik/Wiederverwendbarkeit verschwendest.
>
> Mein Tip: Nimm dir eine Standard-Ringbuffer-Implementierung und such dir
> ein Beispiel wo man sieht, wie diese anzuwenden ist.

Finde deine gefundenen Worte echt nett. Du gehörst in die Politik. Die 
Namenswahl lässt zum Teil zu wünschen übrig. Das stimmt wohl.

Die Funktion heißt so, weil sie mir Bytes aus einen anderen Ringpuffer 
raus saugt.

von Stefan ⛄ F. (stefanus)


Bewertung
0 lesenswert
nicht lesenswert
Hansi schrieb:
> Jetzt verstehe ich es nicht mehr..
> Wie müsste denn jetzt die if Abfrage
> aussehen? Du verunsicherst mich gerade..

Das kann mal nicht so einfach beantworten, da du mehrere Fehler gemacht 
hat. Erkläre erstmal deinen Anwendungsfall.

Wir müssen das nämlich in einen der folgenden Varianten umbauen:

- Ringpuffer
- Zeilenpuffer
- Mehrere Zeilenpuffer

Und wir müssen noch unterschieden, ob Du Text oder Binärdaten empfangen 
willst. Das hast du nämlich alles wild durcheinander gewürfelt.

Wie willst du den Puffer auslesen? Zeilenweise, Zeichenweise oder 
Byte-Für-Byte?

Lies das, bevor du antwortest: 
https://www.mikrocontroller.net/articles/FIFO

Bis morgen...

: Bearbeitet durch User
von guest (Gast)


Bewertung
0 lesenswert
nicht lesenswert
Stefanus F. schrieb:
> Aber in deiner Funktion oben hast du die Größe des Zeigers abgefragt.

Woher willst Du wissen, ob inStream an dieser Stelle als Zeiger 
aufgefasst wird? Aus dem geposteten Codefragment geht das jedenfalls 
nicht hervor. Wenn da "sizeof(stream)" (ohne "in" und mit kleinem 
's')stehen würde hättest Du definitiv Recht, aber so? Wozu er den 
Bereich dann allerdings noch als Funktionsparameter braucht, ist genauso 
unklar.

von Hansi (Gast)


Bewertung
0 lesenswert
nicht lesenswert
Ich benutze hier aus dem Forum die UART Lib. Dort ist ein Ringpuffer 2n 
implementiert.

Da ich auf diesen nicht direkt von meinem Source drauf zu greifen kann, 
habe ich mir diese Funktion gebaut.
Diese liest solange ein bis die Zeichenfolge "\r\n" eintrifft.
Die ganzen anderen Bytes werden auch als die gehändelt. Es gehen also 
außer diese beiden Zeichen keine anderen lesbaren Zeichen ein.

Der Speicher für die ganzen gelesen Zeichen aus dem externen Ringpuffer 
landen in meinem Array. Wenn die erwartete Zeichenfolge wie beschrieben 
eingetroffen ist, gebe ich einen Zeiger auf dessen Anfang zurück. Wenn 
nicht "null"

von Stefan ⛄ F. (stefanus)


Bewertung
0 lesenswert
nicht lesenswert
Hansi schrieb:
> Diese liest solange ein bis die Zeichenfolge "\r\n" eintrifft.
> Die ganzen anderen Bytes werden auch als die gehändelt. Es gehen also
> außer diese beiden Zeichen keine anderen lesbaren Zeichen ein.

Mach mal langsam. Das versteht kein Mensch.

Ein kleiner Tipp: Wenn du Zeilenweise aus einem bestehenden Ringpuffer 
lesen willst, dann implementiere den stdin Stream mittels 
FDEV_SETUP_STREAM und benutze die Standard C Funktionen 
(https://www.nongnu.org/avr-libc/user-manual/group__avr__stdio.html) zum 
Auslesen, zum Beispiel gets().

Von meiner HelloWorld Vorlage kannst du abgucken: 
http://stefanfrings.de/avr_hello_world/index.html

von Experte (Gast)


Bewertung
0 lesenswert
nicht lesenswert
Eigentlich brauchst du einen Zustandsautomaten, der solange Zeichen in 
einen Buffer einliest, bis \r und dann \n empfangen wurde und dann \r 
und \n abschneidet, den String \0-terminiert und zurückgibt.

Wenn der Buffer voll ist bevor \r und \n empfangen wurde, müssen solange 
alle Zeichen verworfen werden bis \r und \n empfangen wurde.
Danach geht es von vorne los.

Der Zustandsautomat kann meinetwegen aus einem Uart-Ringbuffer 
zeichenweise(!) auslesen. Er kann dann meinetwegen die einzelnen Strings 
wieder in einen Ringbuffer schreiben, der String-Objekte aufnehmen kann.

Oder auch anders, ich kenne ja seinen Anwendungsfall nicht. Aber dieses 
Zusammengemansche verschiedener Konzepte und Anfoerderungen ohne jede 
Abstraktion/Generik ist einfach nur schlecht.

von Floppy (Gast)


Bewertung
0 lesenswert
nicht lesenswert
Hansi, lass dich nicht verwirren. Wenn "inStream" ein Array von 1024 
Bytes ist, dann liefert "sizeof(inStream)" die 1024 zurück. Wer etwas 
anderes meint, sollte die Weihnachtszeit nutzen um ein paar Lektionen 
C/C++ zu wiederholen.

Wenn du die Funktion ausschließlich mit "inStream" nutzt, dann ist der 
Übergabeparameter "*stream" hier überflüssig und verwirrend. Wenn du die 
Funktion aber mit verschiedenen Buffern "inStream1, inStream2, .." 
nutzen möchtest, dann solltest du jeweils die Größe der Buffer beim 
Aufruf mitübergeben, und diesen Parameter dann anstelle von 
"sizeof(inStream)" benutzen.

Wenn der Buffer 1024 Bytes gross ist, dann darf "index" maximal 1023 
sein sonst schreibst du hinter den Buffer. Es gibt 2 Stellen in deiner 
Funktion die du ändern solltest.

Hansi schrieb:

>
1
>   if ( last == '\r' && c == '\n' )
2
>   {
3
>     if ( (index - 1) > sizeof(inStream ) )   <-- ALT
4
      if ( index > sizeof(inStream ) )         <-- NEU
5
Hier darf index == 1024 sein, da ja nur auf "index -1" geschrieben wird (\r durch \0 im Buffer ersetzt)
6
>     {
7
>       index = 0;
8
>       return NULL;
9
>     }
10
>     else
11
>     {
12
>       *( stream + ( index - 1 ) ) = '\0';
13
>       index = 0;
14
> 
15
>       return stream;
16
>     }
17
>   }last = c;
18
> 
19
>   if ( index > sizeof(inStream) )     <-- ALT
20
    if ( index >= sizeof(inStream) )    <-- NEU
21
Hier ist index == 1024 schon im verbotenen Bereich
22
>   {
23
>     index = 0;
24
>   }
25
>   else
26
>   {
27
>     *( stream + index++ ) = (uint8_t)c;
28
>   }
29
> 
30
>   return NULL;
31
> }
32
>

Beitrag #5650353 wurde vom Autor gelöscht.
von Stefan ⛄ F. (stefanus)


Bewertung
1 lesenswert
nicht lesenswert
Ich muss mich entschuldigen, ich habe

> sizeof(inStream)    (=1024)

mit

>sizeof(stream)       (=2 oder 4)

verwechselt. Wenn man der Funktion einen Zeiger auf (irgendeinen) Puffer 
übergibt, macht es wenig Sinn, innerhalb der Funktion die Größe des 
konkreten inStream Puffers zu benutzen. Denn der übergebene und 
tatsächlich befüllte Puffer könnte ja ein anderer sein (als inStream), 
uns somit auch eine andere Größe haben.

Diese Unstimmigkeit hat mich verwirrt. Ich habe dachte es sei nur ein 
offensichtlicher Rechtschreibfehler (daher mein Trollvorwurf and 
Floppy), aber es war wohl ein grober Designfehler.

Entweder verzichtest du auf den Funktionsparameter, dann ist die 
Funktion fest an den einen inStream Puffer und seine Größe gebunden, 
oder du übergibst einen beliebigen Puffer als Parameter, brauchst dann 
aber einen zweiten Parameter, der dessen Größe angibt.

Die Größe des Pointers kannst du jedenfalls nicht gebrauchen.

Ich stimme Floppy und Walter S also nun doch zu.

: Bearbeitet durch User

Antwort schreiben

Die Angabe einer E-Mail-Adresse ist freiwillig. Wenn Sie automatisch per E-Mail über Antworten auf Ihren Beitrag informiert werden möchten, melden Sie sich bitte an.

Wichtige Regeln - erst lesen, dann posten!

  • Groß- und Kleinschreibung verwenden
  • Längeren Sourcecode nicht im Text einfügen, sondern als Dateianhang

Formatierung (mehr Informationen...)

  • [c]C-Code[/c]
  • [code]Code in anderen Sprachen, ASCII-Zeichnungen[/code]
  • [math]Formel in LaTeX-Syntax[/math]
  • [[Titel]] - Link zu Artikel
  • Verweis auf anderen Beitrag einfügen: Rechtsklick auf Beitragstitel,
    "Adresse kopieren", und in den Text einfügen




Bild automatisch verkleinern, falls nötig
Bitte das JPG-Format nur für Fotos und Scans verwenden!
Zeichnungen und Screenshots im PNG- oder
GIF-Format hochladen. Siehe Bildformate.
Hinweis: der ursprüngliche Beitrag ist mehr als 6 Monate alt.
Bitte hier nur auf die ursprüngliche Frage antworten,
für neue Fragen einen neuen Beitrag erstellen.

Mit dem Abschicken bestätigst du, die Nutzungsbedingungen anzuerkennen.