String zu Integer (Wer findet den Fehler?)

[Gast (Gast)]

Hallo!

Ich bekomme über eine User-schnittstelle einen String. In erster Instanz 
verifiziere ich die Zeichen, sodass nur noch max. 5 Zahlen und evtl. ein 
Minuszeichen am Anfang enthalten sind.

z.B. "-12345" oder "12345"

Der Wertebereich soll den int-Datentyp entsprechen. D.h. Zahlen zw. 
-32768 und 32767 sollen gültig sein.

D.h. die Zahlen "-55555" oder "66666" sollen einen Fehler zurückgeben. 
(=Funktions-Return -1)

Die Funktion lautet folgendermassen:

char dezstr_2_int( unsigned char *string, int *value )
{
  unsigned char is_neg = 0, i;
  long int ret_value = 0;

  for( i = 0; i < 7; i++)
  {
    if(*(string + i) == 0x00) { break; }
  }

  for(; *string; string++ ) // string-to-integer - Umwandlung
  {
    if (*string == '-') { i--; is_neg = 1; continue; } // Vorzeichen auf negativ
    
    //Dezimalzahl umwandeln
    switch (i)
    {
      case 1: ret_value += (*string - 0x30); break;
      case 2: ret_value += ((*string - 0x30) * 10); break;
      case 3: ret_value += ((*string - 0x30) * 100); break;
      case 4: ret_value += ((*string - 0x30) * 1000); break;
      case 5: ret_value += ((*string - 0x30) * 10000); break;
    }
    i--;
  }

  if (is_neg) // in negative Zahl umwandeln
  {
    ret_value *= (-1);
  }

  //Bereich von Dezimalzahl prüfen und zurückgeben
  if (ret_value > -32769 && ret_value < 32768)
  {
    *value = (int)ret_value; // Zahl zurückgeben
    return 0;
  }

  return -1;
}

Solange der Wert zw. -39999 und 39999 schwankt, macht die Funktion, was 
sie soll. Liegt der Wert ausserhalb dieser Grenze kommt nur Müll raus 
(anstatt eines Fehlers '-1').

Irgendwie hab ich Tomaten auf den Augen, weil ich den Bug nicht finde.. 
:(

Bitte um Hilfe. - Danke!

[Kai G. (runtimeterror)]

In meinen Augen liefert "((*string - 0x30) * 10000)" einen Überlauf ab 
40000

[Timmo H. (masterfx)]

Geht bei mir wunderbar. Du musst natürlich auch auf den Fehler prüfen. 
In Value steht im Fehlerfall eine zufällige Zahl:

int main(int argc, char* argv[])
{
  int test;
  char test1[] = "40450";
  if(dezstr_2_int(test1,&test) == -1)
    printf("Fehler");  
  else
    printf("\n%d", test);

return 0;
}

[Karl H. (kbuchegg)]

Denk mal über folgendes nach:

   ...
   ... Prüfung auf Gültigkeit
 
   if( *string == '-' )
     is_neg = 1;

   ret_val = 0;
   while( *string )
     ret_val = 10 * ret_val + ( *string++ + '0' )

   if( is_neg )
     ret_val = -ret_val;

   ... Prüfung auf Wertebereich

[Karl H. (kbuchegg)]

Timmo H. wrote:
> Geht bei mir wunderbar.

Hast du auf einem PC oder auf einem AVR getstet?

Kai hat das Problem schon richtig erkannt.
Auf einem AVR ist sizeof(int) == 2 und daher wird die
Multiplikation mit 10000 für viele Zahlen überlaufen.

Um das zu  umgehen, muss die Multiplikation als long gemacht werden.

    ((*string - 0x30) * 10000L);

Auf einem PC wirst du das Phänomen whrscheinlich so nicht sehen, da dort 
sizeof(int) heutzutage meistens 4 ist.

[Stefan E. (sternst)]

Karl heinz Buchegger wrote:

   if( *string == '-' )
     is_neg = 1;

->

   if( *string == '-' ) {
     is_neg = 1;
     string++;
   }

[Karl H. (kbuchegg)]

Stefan Ernst wrote:
> Karl heinz Buchegger wrote:
>

>    if( *string == '-' )
>      is_neg = 1;
>

>
> ->
>
>
>

>    if( *string == '-' ) {
>      is_neg = 1;
>      string++;
>    }
>

und wieder mal die alte Weisheit:
Sei der Code auch noch so einfach - vor dem Posten testen!

Danke für den Catch.

und noch einer:

     ret_val = 10 * ret_val + ( *string++ + '0' )

--->

     ret_val = 10 * ret_val + ( *string++ - '0' )

[Ggg (Gast)]

hmm

Aber ret_value hat er doch mit long int definiert - D.h. 4 Byte 
lang..?!?

[Gast (Gast)]

@Ggg: Ja, das hab ich, ABER

karl heinz hat meinen 'Bug' (glaub ich jedenfalls, kann es momentan 
nicht testen) entdeckt -> Multiplikation ist mit einem Int-Wert, anstatt 
mit einem long int..! ;-)

Ich meld mich wieder, ob es das war..

Danke!

[Johannes M. (johnny-m)]

Ggg wrote:
> Aber ret_value hat er doch mit long int definiert - D.h. 4 Byte
> lang..?!?
Ja und? Die Zuweisung wird als letztes ausgeführt (nach der Berechnung). 
Zum Zeitpunkt der Berechnung ist es dem Compiler piepegal, was links von 
dem "=" steht. Wenn nichts anderes angegeben ist, wird die Berechnung 
also in int durchgeführt, solange alle beteiligten Operanden in int 
darstellbar sind. Und in diesem Falle sind alle beteiligten Operanden in 
int darstellbar, weshalb die Berechnung auch in int durchgeführt 
wird.

[Karl H. (kbuchegg)]

Ggg wrote:
> *hmm*
>
> Aber ret_value hat er doch mit long int definiert - D.h. 4 Byte
> lang..?!?

Und, wen kümmerts?

Das ist ein oft gemachter Irrglaube.
Der Datentyp der Ergebnissvariablen ist sowas von irrelevant! Wenn der 
Compiler entscheiden muss, wie eine bestimmte Operation durchzuführen 
ist, dann sieht er sich die beiden Operanden an! Einzig und alleine 
deren Datentyp entscheidet.

    3 / 5

3 ist ein int. 5 ist ein int. Also wird die Division als int-Division 
ausgeführt. Das du das Ergebnis

   double i = 3 / 5;

in einem double ablegst, ist für die Entscheidung wie die Division 
durchzuführen ist, nicht von Belang.

[SoLaLa (Gast)]

> double i = 3 / 5;

wäre also danach i=0 ?

[Sven P. (Gast)]

Ja.

Und: long int --> uintXX_t, kommt oft sicherer. Spricht sonst noch was 
gegen atoi und Konsorten?

[Timmo H. (masterfx)]

@Karl heinz Buchegger
>Timmo H. wrote:
>> Geht bei mir wunderbar.
>Hast du auf einem PC oder auf einem AVR getstet?

Auf einem PC, hier steht ja nicht dass es sich um ein AVR handelt. Es 
ist ja allgemein GCC-Forum und nicht AVR GCC. Im Ausgangsposting steht 
nirgends AVR.

[Gast (Gast)]

=> Funktioniert  :-)

Danke nochmals..!

[Jörg W. (dl8dtl) (Moderator)]

Wenn du sowieso zwischendrin einen long int brauchst, warum nimmst
du dann nicht gleich strtol()?  Dann hätteste dir den ganzen Stress
gespart... (OK, so hast du dafür was fürs Leben gelent. :)