[Glaskugel] Funktionsaufruf lässt Controller hängen

[D a v i d K. (oekel)]

Hi,

wie die Überschrift vermuten lässt erwarte ich keine Wunder.
Aber vielleicht fällt dem ein oder anderem etwas dazu ein?
(Falls nicht bitte einfach weiter gehen :P)

...wieso der folgende Code beim einmaligen und zweimaligen Aufruf 
passiert werden kann, jedoch bei 3-maligem Aufruf den Atmega16 zum 
hängen bringt.
(Meine Annahme, dass er hängen bleibt beruht auf der Ausgabe auf einem 
2x16 Zeilen-Display bzw. der NICHT Ausgabe der sofort folgendem 
Zeilenausgabe auf demselbigen.)

//mein "Werk"
void lcd_printInt(uint16_t value, uint8_t c, bool leadingZeros) {
  char str[c];
  utoa(value, str, 10);
  if (leadingZeros) {
    for (uint8_t i = getDigitCount(value); i < c; i++)
      lcd_print_P(PSTR("0"));
  }
  lcd_print(str);
}

//-  Print string to cursor position
void lcd_print(char *string) {

  while(*string)  {
    lcd_putchar(*string++);
  }
}
//-  Put char to atctual cursor position
void lcd_putchar(char lcddata) {

  lcd_write((lcddata >> 4) | CMD_RS);
  lcd_write((lcddata & 0x0F) | CMD_RS);
}

Mein Aufruf:

#define IN2 !(PINA & (1<<PA6))//PIN34
#define JP1 !(PINA & (1<<PA0))//PIN39
#define JP2 !(PINA & (1<<PA1))//PIN40

lcd_printInt(JP1, 1, false);
lcd_printInt(JP2, 1, false);
lcd_printInt(IN2, 1, false);

Mein Workarround:

uint8_t combi = JP1*100+JP2*10+IN2;
lcd_printInt(combi, 3, true);

Grüße Oekel

[Felix U. (ubfx)]

D a v i d K. schrieb:
> char str[c];

also in c bytes kannst du schon mal nicht c Zeichen plus 1 Nullbyte für 
die Terminierung unterbringen. Vielleicht liegt es daran?

[Jim M. (turboj)]

Deine lcd_printInt hat einen Buffer Overflow bei

char str[c];

Der muss das ganze Ergebnis von utoa inklusive nachfolgender Null 
enthalten können.

[Kaj G. (Firma: RUB) (bloody)]

Du machst da ja auch ganz grossen Mist.

#define IN2 !(PINA & (1<<PA6))//PIN34


lcd_printInt(JP1, 1, false);  // <-- !!!


void lcd_printInt(uint16_t value, uint8_t c, bool leadingZeros) {
  char str[c];               // <-- !!!
  utoa(value, str, 10);      // <-- !!!
  if (leadingZeros) {
    ...
  }
  lcd_print(str);
}


void lcd_print(char *string) {
  while(*string)  {          // <-- !!!
    ...
  }
}

Schau dir mal die Stellen an, die ich markiert habe...
Dazu noch das hier:
http://www.atmel.com/webdoc/AVRLibcReferenceManual/group__avr__stdlib_1gad50d835af7e358518fd100179732e948.html

The function utoa() converts the unsigned integer value from val into
an ASCII representation that will be stored under s. The caller is
responsible for providing sufficient storage in s.


Note

The minimal size of the buffer s depends on the choice of radix. For
example, if the radix is 2 (binary), you need to supply a buffer with a
minimal length of 8 * sizeof (unsigned int) + 1 characters, i.e. one
character for each bit plus one for the string terminator. Using a larger
radix will require a smaller minimal buffer size.


Warning

If the buffer is too small, you risk a buffer overflow.

[zer0 (Gast)]

"Zeilenausgabe" - Hat das Ding Scrolling eingebaut oder sowas? Auf einem 
2-Zeilen display verschwindet die 3. sonst vermutlich im Nirvana.

Ansonsten: Die Timings für das Ding alle penibel beachtet? Sonst kann 
schon mal irgendetwas verschluckt werden - irgendwann...

Wenn der Code echt irgendwo hängen bleibt würde ich tippen, er wartet 
auf ein Zeichen vom LCD, das niemals kommt.

[Axel S. (a-za-z0-9)]

Jim M. schrieb:
> Deine lcd_printInt hat einen Buffer Overflow bei
>
>

char str[c];

>
> Der muss das ganze Ergebnis von utoa inklusive nachfolgender Null
> enthalten können.

Nicht zu vergessen der Irrsinn, einen solchen Puffer überhaupt mit 
dynamischer(!) Länge anzulegen. Ein uint16_t hat maximal 5 
Dezimalstellen, dazu noch das terminierende \0, macht einen Buffer Länge 
6. Den alloziert man einmal, vorzugsweise als

static char str[6];

und nicht bei jedem Aufruf der Funktion wieder.

Mal wieder ein Traumbeispiel für den immer "korrekten Code" des TE.

[zer0 (Gast)]

Felix U. schrieb:
> D a v i d K. schrieb:
>> char str[c];
>
> also in c bytes kannst du schon mal nicht c Zeichen plus 1 Nullbyte für
> die Terminierung unterbringen. Vielleicht liegt es daran?

Wenn man das so betrachtet klingt das plausibel. Allerdings - was sollte 
da denn auf dem Stack kaputt gehen, was 2 mal aber nicht 3 mal gut geht? 
Schreibt doch immer eine 0 also müsste alles deterministisch ablaufen...

[Stefan E. (sternst)]

zer0 schrieb:
> Allerdings - was sollte
> da denn auf dem Stack kaputt gehen, was 2 mal aber nicht 3 mal gut geht?

Die Return-Adresse.

[zer0 (Gast)]

Stefan E. schrieb:
> zer0 schrieb:
>> Allerdings - was sollte
>> da denn auf dem Stack kaputt gehen, was 2 mal aber nicht 3 mal gut geht?
>
> Die Return-Adresse.

Und deshalb erscheint nichts auf dem Display?

[zer0 (Gast)]

zer0 schrieb:
> Stefan E. schrieb:
>> zer0 schrieb:
>>> Allerdings - was sollte
>>> da denn auf dem Stack kaputt gehen, was 2 mal aber nicht 3 mal gut geht?
>>
>> Die Return-Adresse.
>
> Und deshalb erscheint nichts auf dem Display?

Ach so, nach dem 2. ist er im Nirvana... Das könnte durchaus sein.

[D a v i d K. (oekel)]

Axel S. schrieb:

> Mal wieder ein Traumbeispiel für den immer "korrekten Code" des TE.
Diese Aussage kreidest du mir wohl jetzt noch eine Weile an? :)

Ich bin nicht perfekt aber ebenso wenig nun eingeschnappt, daher gerne 
mehr davon, solange ich gleichzeitig viele hilfreiche Infos bekomme.

Nur SO lerne ich etwas. Danke.

[MaWin (Gast)]

Vom Überlauf abgesehen wird der Code effizienter, wenn du den Puffer 
nicht dynamisch, sonder statisch auf dem Stack alloziierst. Als Größe 
einfach die Anzahl der Ziffern der größten Zahl, die utoa ausgeben kann, 
plus eins für NUL.