Ein Stringproblem

[Hemi 8. (hemi850)]

Hallo Zusammen,

ich versuche gerade ein String zusammen zu bauen:

Hier die Funktion:

// Eine Nachricht zusammenbauen:
char build_ibus_message ( char Sender, char Receiver, char Message) {
  unsigned char ibus_message[127]  ;
  unsigned char length = 0;

  length = strlen (Message);

  strcpy (ibus_message, Sender);
  strcat (ibus_message, length);
  strcat (ibus_message, Receiver);
  strcat (ibus_message, Message);

  return ibus_message;
}

Sender, Receiver und Message sind jeweils ein "array of chars".

Mein Compiler sagt mir dann:

../help_funcs.h: In function 'build_ibus_message':
../help_funcs.h:8: warning: passing argument 1 of 'strlen' makes pointer from integer without a cast
../help_funcs.h:10: warning: pointer targets in passing argument 1 of 'strcpy' differ in signedness
../help_funcs.h:10: warning: passing argument 2 of 'strcpy' makes pointer from integer without a cast
../help_funcs.h:11: warning: pointer targets in passing argument 1 of 'strcat' differ in signedness
../help_funcs.h:11: warning: passing argument 2 of 'strcat' makes pointer from integer without a cast
../help_funcs.h:12: warning: pointer targets in passing argument 1 of 'strcat' differ in signedness
../help_funcs.h:12: warning: passing argument 2 of 'strcat' makes pointer from integer without a cast
../help_funcs.h:13: warning: pointer targets in passing argument 1 of 'strcat' differ in signedness
../help_funcs.h:13: warning: passing argument 2 of 'strcat' makes pointer from integer without a cast
../help_funcs.h:15: warning: return makes integer from pointer without a cast
../help_funcs.h:15: warning: function returns address of local variable

Was will er mir damit sagen?

ich stehe auf dem Schlauch.

Danke schonmal.

Grüsse
Heinrich

[daniel (Gast)]

Moin,

du solltest die Typen der Parameter in char*
ändern.

Gruß
Daniel

[holger (Gast)]

char build_ibus_message ( char *Sender, char *Receiver, char *Message) {

[Johannes M. (johnny-m)]

Heinrich Geiger wrote:
> char build_ibus_message ( char Sender, char Receiver, char Message) {
> [...]
> Sender, Receiver und Message sind jeweils ein "array of chars".
Das ist doch schon gelogen! Alle drei Variablen sind schlicht char, nix 
Array. Wenn Du Arrays übergeben willst, dann musst Du schon Zeiger auf 
char als Parameter benutzen...

Außerdem ist ibus_message vom Typ unsigned char , die Funktionen 
wollen aber char sehen, weshalb der Compiler zu Recht meckert, dass es 
Vorzeichenkonflikte gibt.

[daniel (Gast)]

Außerdem gibt es da noch ein Problem:

der Rückgabewert sollte wohl auch ein zeiger auf eine
Zeichenkette sein (char*).

In deinem Fall ist ibus_message aber ein lokales Array.
Wenn du einen Zeiger auf dieses Objekt zurückgibst, wird
der Zeiger ungültig, sobald die Funktion verlassen wird.

Verwende stattdessen ein lokales, als 'static' markiertes Array oder
gar eine globale Variable, auch wenn es nicht schön ist.


Zudem verwendest du unsigned char und char durcheinander.

Gruß
Daniel

[holger (Gast)]

>  unsigned char ibus_message[127]  ;

Ein static könnte da auch nicht schaden.
Sonst ist der Inhalt von ibus_message[]
ganz schnell im Eimer.

[Rolf Magnus (Gast)]

Mit

> warning: passing argument 1 of 'strlen' makes pointer from integer
> without a cast

will dir der Compiler sagen, daß strlen einen Zeiger erwartet, du ihm 
aber einen Integer übergibst (char ist ein Integertyp).


Und mit

> warning: pointer targets in passing argument 1 of 'strcpy' differ in
> signedness

will er dir sagen, daß ein ibus_message nicht, wie von strcpy verlangt, 
ein Array aus char, sondern ein Array aus unsigned char ist.

[holger (Gast)]

Das wird wahrscheinlich auch nicht funktionieren:

  unsigned char length = 0;
  length = strlen (Message);

  strcat (ibus_message, length);

[Hemi 8. (hemi850)]

So, es funzt:

char ibus_message[127];

// Eine Nachricht zusammenbauen:
void build_ibus_message ( char *Sender, char *Receiver, char *Message) {
  
  char length = 0;

  length = strlen (Message);

  strcpy (ibus_message, Sender);
  strcat (ibus_message, &length);
  strcat (ibus_message, Receiver);
  strcat (ibus_message, Message);
}

Danke & Grüsse
Heinrich

[holger (Gast)]

Die Zeile ist immer noch faul

  strcat (ibus_message, &length);

length ist kein String. strcat kann unter Umständen
einmal quer durch den Speicher jagen und alles was nicht 0 ist
an ibus_message[] anfügen. Da die Größe von ibus_message[]
nicht berücksichtigt wird sogar darüber hinaus. Und dann wird
es richtig nett.

[Hemi 8. (hemi850)]

Wie kann ich es dann besser machen?

Danke.

[daniel (Gast)]

Ein Integer-Typ (int, aber auch char) ist eine Zahl - was du brauchst 
ist
eine Zeichenkette (zeiger auf ein char array), die die Darstellung der 
Zahl enthält.

Wenn dir die Funktion itoa() zur verfügung steht,
kannst du sie verwenden, um eine Zahl in eine Zeichenkette
umzuwandeln.

...
/* Du musst sicher sein, dass nie Zahlen umgewandelt werden, die
mehr als 16 Stellen haben. Das sollte bei 32-bit immer gegeben sein. */
#define MAX_NUMBER_STR_LEN 16
/* Am besten keine absoluten Zahlen mitten im Code.
Das würde die Wartbarkeit verringern. */
#define IBUS_MSG_LEN       128
char ibus_message[IBUS_MSG_LEN];
...
// Eine Nachricht zusammenbauen:
void build_ibus_message( char *Sender, char *Receiver, char *Message)
{
  size_t length = 0;
  char lenStrBuf[MAX_NUMBER_STR_LEN];

  length = strlen (Message);

  /* Basis=10 -> Dezimaldarstellung */
  /* Basis=16 -> Hexadezimaldearstellung */
  itoa(length, lenStrBuf, 10);

  strncpy (ibus_message, Sender, IBUS_MSG_LEN);
  strncat (ibus_message, lenStrBuf, IBUS_MSG_LEN);
  strncat (ibus_message, Receiver, IBUS_MSG_LEN);
  strncat (ibus_message, Message, IBUS_MSG_LEN);
  return;  
}

Beachte bitte:
- strncpy, strncat statt strcpy, strcat
  Die maximale Länge von ibus_message wird geprüft.
  -> Sicherheit vor Speicherüberläufen
  Daumenregel: Nie die Varianten ohne 'n' verwenden!
- Wenn build_ibus_message ein zweites mal aufgerufen wird,
  wird die erste message überschrieben.

Sauber und sicher kannst du build_ibus_message so verwenden:

void any_function_name(void)
{ 
  ...
  char myMsg1[IBUS_MSG_LEN];
  char myMsg2[IBUS_MSG_LEN];
  ...
  build_ibus_message(...);
  strncpy(myMsg1, ibus_message, IBUS_MSG_LEN);
  build_ibus_message(...);
  strncpy(myMsg2, ibus_message, IBUS_MSG_LEN);
  ...
}

Gruß
Daniel

[Hemi 8. (hemi850)]

Hallo Daniel,

vielen Dank für Deine ausführliche Antwort. Danke.

Ich habe in der Zwischenzeit es versucht so zu lösen:

#include <stdlib.h>
#include <stdio.h>
#include <string.h>

struct transfer {
  char sender;
  char length;
  char receiver;
  char message[9];
  char xor;
};


struct transfer transfer;

int main ( void ) {

  char msg[] = {0xBE, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF};
  int i;

  transfer.sender = 0x02;
  transfer.length = strlen (msg)+2;
  transfer.receiver = 0x01;
  
  memcpy (transfer.message, msg, sizeof(msg));

  transfer.xor = 0;  

  printf ("Länge %X\n", transfer.length);
  
  transfer.xor ^= transfer.sender;
  transfer.xor ^= transfer.length;
  transfer.xor ^= transfer.receiver;

  for (i=0; i<sizeof(msg); i++) {
    transfer.xor = transfer.xor ^ msg[i];
  }
  
  
  printf ("XOR: %X\n", transfer.xor);

  printf ("Sender: %X, Länge: %X, Empfänger: %X, Daten: %X, CRC: %X\n", transfer.sender, transfer.length, transfer.receiver, transfer.message[0], transfer.xor);


  return 0;
}

Diese Funktion hat aber npaar Probleme:

1. Strlen ist zwar schön und gut, aber es bricht ab, sobald sie ein 0x00 
"sieht", weil das bedeutet für sie Stringende. In meinem Fall ist aber 
ein 0x00 zulässig.

Das würde bedeuten, dass ich immer wissen muss, wie lange meine 
Netzdaten sind, dann kann ich sauber arbeiten.

Das Gleiche gilt auch für SizeOf, so viel ich weiss.

Alle Daten sind dexadzimal, nicht dezimal.

Hier mal ein Beispiel von einer Nachricht:

char nachricht = {0x02, 0x0B, 0x01, 0xBE, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xB6};

0x02 ist mein Sender (1 Byte lang)
0x0B ist die Länge ( also 11 Byte), Länge an sich ist auch ein Byte
0x01 ist mein Empfänger (1 Byte lang)
0xBE, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF sind meine 
Nutzdaten (in diesem Fall 9 Byte von 32 möglichen)
0xB6 ist XOR-CRC (1 Byte)

Die Länge wird über Nutzdaten + Empfänger + XOR CRC ausgerechnet.
XOR wird über die gesamte Nachricht ausgerechnet.

Das Problem, was ich noch habe, dass ich die transfer.message nicht 
dynamisch machen kann, da ich nicht weiss, wie lang der String sein 
soll. Bzw. es ist von Fall zu Fall unterschiedlich. Deswegen muss ich 
die Länge der Nutzdaten vor dem Bau der Nachricht schon wissen, was aber 
kein Problem ist.

Grüsse
Heinrich

[daniel (Gast)]

Moin Heinrich,

bitte durchdenk' mal die folgenden Fragen/Anmerkungen,
heute abend/nacht werde ich noch mal reinschauen,
jetzt muss ich erstmal weg.

- brauchst du build_ibus_message() dann gar nicht mehr?
  oder soll der o.g. code das ersetzen?
- sizeof() und strlen() sind grundverschieden, bitte nachschlagen!
- Bei der Deklaration von transfer.message verwendest du 9 und nicht 32.
- Wenn es kein Problem ist, dass du die Länge der Nutzdaten vorher 
kennst,
  was ist dann dein Problem? Setz' die arraygröße von transfer.message
  auf das maximum (32) und verwende die bekannte message-Größe als 
parameter für memcpy().
- mit printf("%X", transfer.message[0]); gibst du nur das erste Zeichen 
der Msg aus.

Ansonsten ist der Code doch brauchbar:

Vorschlag:

#define HEADER_LEN   2 // receiver + xor
#define MAX_PAYLOAD_LEN  32 
#define MAX_IBUS_MSG_LEN (MAX_PAYLOAD_LEN+HEADER_LEN)
char ibus_msg[MAX_IBUS_MSG_LEN];
// Gibt die Gesamtlänge zurück
unsigned char build_msg(struct transfer *t)
{
  unsinged char i = 0, j;
  if (!t) return;
  memset(ibus_msg, 0, MAX_IBUS_MSG_LEN);
  ibus_msg[i++] = t->sender;
  ibus_msg[i++] = t->total_length;
  ibus_msg[i++] = t->receiver;
  memcpy(&(ibus_msg[i]), t->payload, t->payload_length);
  i += t->payload_length;
  ibus_msg[i++] = t->xor;
  return i;
}

struct transfer musst du dann anpassen und ausfüllen.

Gruß
Daniel

[Hemi 8. (hemi850)]

Hallo Daniel,

danke erstmal.

zum Punkt 1: Doch diese Funktion brauche ich nach wie vor. Ich mache mir 
immer so kleine Programmchen, mit denen ich es dann austeste und diese 
dann in das Programm einbaue. Also es wäre dann Ersatz.

zum Punkt 2: Stimmt, sizeof liefern die reservierte Grösse zurück und 
strlen die Länge eines Strings. Hast Recht.

Zum Punkt 3: Ja, das habe ich testweise runtergesetzt. Es hat den 
Hintergrund, dass nicht immer alle 32 Byte belegt werden, aber dann alle 
32 Byte verschickt werden, wenn man die Grösse so belässt.

Zum Punkt 4: Ja, die Länge der Nachricht weiss ich schon vorher. Das 
heisst, wenn ich zum Beispiel Nutzdaten als "92 95 83" habe, dann weiss 
ich ja, dass diese Geschichte ja nur 3 Byte lang ist, dazu kommt noch 1 
Byte für Empfänger und 1 Byte für CRC, ergibt also 5 Byte. Diese 5 Byte 
kann ich dann schon mal reinschreiben als Länge.

Zum Punkt 5: Ja, das weiss ich, war testweise geändert.

Ich glaube, ich bohre einfach meine Sendenfunktion so weit auf, dass ich 
diese Parameter an sie übergebe und sie dann die ganzen Daten 
rausschickt und dann am Ende die XOR CRC ausrechnet. Ich denke, das wäre 
das Einfachste.

Oder wie siehst Du das?

Danke & Grüsse
Heinrich

[Daniel (Gast)]

Ja, ich denke auch, dass deine Sendefunktion das ganz gut
komplett erledigen kann. So kompliziert ist es ja nicht mehr,
wenn du die erforderlichen Parameter ohnehin im Voraus kennst.

Gruß
Daniel

[Hemi 8. (hemi850)]

Ich wollte halt eine "generische" Funktion haben, die mir die 
Nachrichten zusammenbaut. Aber das ist ja eigentlich nicht erforderlich, 
da man sie beim Bauen ja auch gleich rauschicken kann.

Trotzdem vielen Dank.

Grüsse
Heinrich.