Code funktioniert- Verbesserungsvorschlag?

[Stephan (Gast)]

Hallo!

Ich möchte, dass der Controller die Werte eines Strings einem Variablen- 
Array korrekt zuweist.

Der String setzt sich aus Teistrings zusammen, die mit Semikolon 
getrennt sind. Ein Teilstring ist mit Doppelpunkt getrennt.

Stringbeispiel: 1:54;3:23;8:45

Daraus soll folgen, dass
receive_value[1] 54,
receive_value[3] 23 und
receive_value[8] 45 annimmt.

Mein Code dazu funktioniert, allerdings habe ich das Gefühl dass der 
ziemlich aufgebläht ist- Stringfunktionen sollen ja recht mühsam sein. 
Ich weiss aber nicht wie ich ihn optimieren kann. Weiß wer Rat?

char receive_string[20][20];
char receive_value[20];
char temp_string[20];

void readout_string(void)
{  
  volatile unsigned char *ptr;
  int i = 0;
  int k = 0;
  int l = 0;
  ptr = strtok(uart0_string, ";");

  while(ptr != NULL) 
  {
    strcpy(receive_string[i], ptr);
    ptr = strtok(NULL, ";");
    i++;
  }
  
  for (k = 0; k < i; k++)
  {
    ptr = strtok(receive_string[k], ":");
    strcpy(temp_string, ptr);
    l = atoi(temp_string);
    ptr = strtok(NULL, ":");    
    strcpy(temp_string, ptr);    
    receive_value[l] = atoi(ptr);
  }  
}

[Karl H. (kbuchegg)]

mach alles in einem Aufwasch, ohne zuerst die Teilstrings in ein 
getrenntes String Array umzukopieren.

Du darfst bei strtok durchaus auch abwechselnd verschiedene Trenner 
benutzen

...

  ptr = strtok( uart_string, ":" );
  while( ptr )
  {
     // jetzt haben wir den "Index" -> in eine Zahlumwandeln
     index = atoi( ptr );
     ptr = strtok( NULL, ";" );
     if( ptr )
     {
       // und jetzte auch noch den zugehörigen Wert
       wert = atoi( ptr );
       receive_value[index] = wert;
     }

     ptr = strtok( NULL, ":" );
   }
 }

sieht doch schon gleich viel kürzer aus und ist auch sicherer, weil du 
nicht gefahr läufst, bei den strcpy die Zielstrings zu überlaufen.
Und 400 Bytes SRAM sparst du obendrein :-)

[Stephan (Gast)]

Cool. Funktioniert immernoch.

[Tom K. (ez81)]

Ene kleine Fehlerquelle:

i = atoi(ein_string_von_aussen);
foo[i] = atoi(ein_anderer_string);

verlagert die Verantwortung für das Einhalten der Arraygrenzen auf das 
Programm, das die Strings sendet. Geht da irgendetwas daneben, wird 
irgendwo wild in den RAM geschrieben.

[Stephan (Gast)]

Tom K. schrieb:
> Geht da irgendetwas daneben...


D.h., wenn StringvonAussen = "100", foo aber nur 50 Elemente groß ist, 
schwappts über, gell?
Wie umgehen? Einfache if-Abfrage?

[chris (Gast)]

nur mit strtol

Stephan schrieb:
> Stringbeispiel: 1:54;3:23;8:45
>
> Daraus soll folgen, dass
> receive_value[1] 54,
> receive_value[3] 23 und
> receive_value[8] 45 annimmt.

unsigned char
parse(char*str) { unsigned char ret;
 do {
     idx=strtol(str,&str,0); // oder 10 fuer dec
     while(*str++==':') {
       val=strtol(str,&str,0); // oder 10 fuer dec
       if(idx>=0&&idx<sizeof(receive_value)/sizeof(receive_value[0]))
         receive_value[idx++]=val,ret++;
     }
    } while(*str++==';');
 return ret;
}

> Stringbeispiel: 1:54:33:44;13:23;8:45
>
> Daraus soll folgen, dass
> receive_value[1] 54,
> receive_value[2] 33,
> receive_value[3] 44,
> receive_value[13] 23 und
> receive_value[8] 45 annimmt.

[chris (Gast)]

ret sollte mit 0 initialisiert sein.

[Stephan (Gast)]

chris schrieb:
> unsigned char
> parse(char*str) { unsigned char ret;
>  do {
>      idx=strtol(str,&str,0); // oder 10 fuer dec
>      while(*str++==':') {
>        val=strtol(str,&str,0); // oder 10 fuer dec
>        if(idx>=0&&idx<sizeof(receive_value)/sizeof(receive_value[0]))
>          receive_value[idx++]=val,ret++;
>      }
>     } while(*str++==';');
>  return ret;
> }

Da muss man erstmal hinterkommen.....

[Karl H. (kbuchegg)]

Stephan schrieb:
> Tom K. schrieb:
>> Geht da irgendetwas daneben...
>
>
> D.h., wenn StringvonAussen = "100", foo aber nur 50 Elemente groß ist,
> schwappts über, gell?
> Wie umgehen? Einfache if-Abfrage?

:-)
Ja, die einfachsten Dinge sind oft die besten.

[Karl H. (kbuchegg)]

Stephan schrieb:
> chris schrieb:
>> unsigned char
>> parse(char*str) { unsigned char ret;
>>  do {
>>      idx=strtol(str,&str,0); // oder 10 fuer dec
>>      while(*str++==':') {
>>        val=strtol(str,&str,0); // oder 10 fuer dec
>>        if(idx>=0&&idx<sizeof(receive_value)/sizeof(receive_value[0]))
>>          receive_value[idx++]=val,ret++;
>>      }
>>     } while(*str++==';');
>>  return ret;
>> }
>
> Da muss man erstmal hinterkommen.....

halb so wild.
strtol vereint mehr oder weniger ein atoi und das strtok in einer 
Funktion.

atoi würde selbsttätig mit dem Konsumieren des Strings aufhören, sobald 
etwas nicht mehr zur Zahl gehört.
D.h. wirfst du atoi das hier vor
"123:56"
dann konsumiert atoi alles bis zum ':'.
Soweit so gut. Nur weißt du nicht, wo im String diese Aufhören 
stattfindet, so dass du das nächste atoi 1 Zeichen dahinter ansetzen 
könntest.

und genau da kommt strtol ins Spiel. Es liefert dir genau diese 
Position, bis zu der es aus dem String Zeichen konsumiert hat. Mit 
dieser Position (nach Prüfung natürlich) kannst du dann den nächsten 
strtol dahinter ansetzen, der wieder alles was nach Zahl aussieht 
konsumiert und dir mit dem Pointer bekannt gibt, wo dieses Ende war. 
Woraufhin du dort den nächsten strtol ansetzt, etc. etc.

[Tom K. (ez81)]

Stephan schrieb:
> schwappts über,

Programme werden zuverlässiger, wenn man solche Eingaben von Anfang an 
als potentiell falsch oder bösartig betrachtet ;)