GNUARM: Problem mit einer Warnung

[Nador R. (rifman)]

Hallo,

wenn ich diese Programmzeile :

CAN_MSG mRxCAN[MAX_QUEUE];
unsigned short mRxOUT
unsigned int *pSrc;

pSrc = (unsigned int *) &(mRxCAN[mRxOUT]);

mit GNUARM Kompiliere bekomme ich immer diese Warnung angezeigt:
"cast increases required alignment of target type"
was mache ich denn Falsch?

in can.h ist CAN_MSG-Struktur so definiert;

#define MAX_QUEUE   20

typedef struct
{
  unsigned int MsgID; 
  unsigned int DatA;  
  unsigned int DatB; 
  unsigned int Frame; 
} CAN_MSG;

Danke.

[Stefan B. (stefan)]

Nador Rif wrote:
> was mache ich denn Falsch?

Vielleicht gar nichts, deshalb ist es auch nur eine Warnung. Es fehlt 
der Code, der zeigt, wie du mit pSrc umgehst.

pSrc ist ein Zeiger auf unsigned int, d.h. wenn du pSrc erhöhst, kommst 
du zum nächsten unsigned int und das ist im Normalfall sizeof(unsigned 
int) Bytes entfernt.

Wenn du von einem Element von mRxCAN zum nächsten gehst, ist der Abstand 
im Normalfall sizeof(CAN_MSG), d.h. der vierfache Wert wie bei pSrc.

Willst du mit pSrc++ also die kompletten Strukturen in mRxCAN ab dem 
Element mRxCAN[mRxOUT] abklappern, könnte das schief gehen, weil du beim 
++ mitten in einer Struktur CAN_MSG landest.

Man müsste also im von GNUARM produzierten Assembler-Listing nachsehen, 
ob der GCC den Code so weit aufgebohrt (alignment increased) hat, dass 
pSrc++ korrekt vier mal sizeof(unsigned int) weiterschaltet. Also 
implizit aus unsigned int *pSrc ein CAN_MSG *pSrc gemacht hat.

Andererseits wenn du mitten in der Struktur landen willst, d.h. mit 
einem Zeiger mitten in einem Element mit Struktur CAN_MSG operieren 
willst, dann könntest du die Zuweisung so machen:

pSrc = (unsigned int *) &(mRxCAN[mRxOUT].MsgID);

Beim Erhöhen mit ++ zeigt dann pSrc auf mRxCAN[mRxOUT].DatA, dann auf 
...DatB und so weiter.

[Martin Thomas (Gast)]

Einen Pointer auf ein Structobject in einen int-pointer casten. Ist aber 
"nur" ein Warning.

Warum nicht so?:

...
CAN_MSG *pSrc;
pSrc = &(mRxCAN[mRxOUT]);
...

Falls die Idee dahinter ist, die Elemente des Structobjekts später als 
"int-Array" anszusprechen: besser nicht so. Solange nur uint32_t im 
struct sind, mag das noch halbwegs funktionieren, ansonsten handelt man 
sich nur Stress mit möglichen Alignments ein. Besser einen Serializer 
schreiben, in dem man ein Array "von Hand" zusammenbaut. Dabei kann man 
dann auch gegf. gleich big/little-Endian berücksichtigen. Für die paar 
Elemente in einem CAN-Packet hällt sich der Laufzeitaufwand in Grenzen. 
Evtl. hilft "packed-attribute" aber das wird dann schlecht portabler und 
fehleranfälliger Code und man muss mglw. mit warnings "leben".

[Nador R. (rifman)]

Danke für eure Antworten, um einen kompletten Überblick zu haben poste 
ich hier den Rest:

void CAN_PullMessage (void)
{
  
  unsigned int *pSrc; // Source pointer
  unsigned char *pDst; // Destination pointer
  
  
  // Initialize pointers
  pSrc = (unsigned int *) &(mRxCAN[mRxOUT]);
  pDst = (unsigned char *) &(uc_RxBuf[0]);
  
  *pDst = *pSrc; // Copy MsgID

   pSrc++;
   pDst += 4;
   *pDst = *pSrc; // Copy DatA
  
   pSrc++;
   pDst += 4;
   *pDst = *pSrc; // Copy DatB
  
   pSrc++;
   pDst += 4;
   *pDst = *pSrc; // Copy frame
  
   Write_Buffer(&uc_RxBuf[0]); 
  
  // Adjust OUT pointer
   mRxOUT++;
  
   if (mRxOUT >= MAX_QUEUE)
    {
  mRxOUT = 0;
    }
}

[Stefan B. (stefan)]

Also diese Variante:

pSrc = (unsigned int *) &(mRxCAN[mRxOUT].MsgID);

Das (und die drei Kollegen davon)

   pSrc++;
   pDst += 4;
   *pDst = *pSrc; // Copy DatA

sieht für mich fischig aus.

Das erzeugt nämlich drei undefinierte Bytes in uc_RxBuf und eine von 
unsigned int runter auf unsigned char massakrierte Kopie.

Die Anmerkungen von Martin, sind übrigens korrekt und wertvoll. Besser 
nicht so arbeiten.

[Nador R. (rifman)]

Martin Thomas wrote:
>  Besser einen Serializer
> schreiben, in dem man ein Array "von Hand" zusammenbaut.

ich habe nicht verstanden was du damit meinst, könntest du das 
vielleicht mehr erklären, ich bin nämlich noch anfänger.

Danke

[Nador R. (rifman)]

Stefan "stefb" B. wrote:

> Das erzeugt nämlich drei undefinierte Bytes in uc_RxBuf und eine von
> unsigned int runter auf unsigned char massakrierte Kopie.

könntest du mir bitte verstänlich machen woher diese undefinierte Bytes 
kommen und warum?Ich bin wie gesagt ein Anfänger, und solche Infos sind 
glaube ich grundsätzlich.

Danke

[Falk B. (falk)]

@ Nador Rif (rifman)

>kommen und warum?Ich bin wie gesagt ein Anfänger, und solche Infos sind
>glaube ich grundsätzlich.

Wenn du Anfänger bist, warum machst du dann solche linken Sachen? Warum 
greifst du nicht ganz normale auf Daten in Strukturen bzw über die 
dazugehörigen Pointer zu wie es im Lehrbuch steht? Dann geht alles 
seinen Gang.

MFG
Falk

[Helge Suess (Gast)]

Hallo!

Der ARM hat eine Eigenheit. Variablen mit 2 oder 4 Byte Grösse (int16_t, 
int32_t etc.) müssen auf Adressen aligned sein, die durch die Grösse der 
Variablen restlos teilbar sind.
Wenn man z.B. Strukturen über den Bus bekommt die dicht gepackt ein Byte 
Länge und dann ein paar word oder int an Nutzdaten haben geht der 
Zugriff mit einem Cast schief wenn das Byte auf einer geraden Adresse 
liegt.

Die einzie Chance ist dann das byteweise Umkopieren in eine dafür 
ausgerichtete neue Struktur. Bei dieser Gelegenheit kann man auch das 
Problem der little und big endians (Wikipedia 
http://de.wikipedia.org/wiki/Little_endian) mit lösen.

Helge ;-)=)

[Nador R. (rifman)]

Falk Brunner wrote:
> @ Nador Rif (rifman)
>
>>kommen und warum?Ich bin wie gesagt ein Anfänger, und solche Infos sind
>>glaube ich grundsätzlich.
>
> Wenn du Anfänger bist, warum machst du dann solche linken Sachen? Warum
> greifst du nicht ganz normale auf Daten in Strukturen bzw über die
> dazugehörigen Pointer zu wie es im Lehrbuch steht? Dann geht alles
> seinen Gang.
>
> MFG
> Falk

das Programm war ein Beispiel, wie man die CAN-Schnittstelle 
programmieren kann,ich habe das Beispiel für meine Applikation 
angepasst(oder dacht ich zumindest)

[Karl H. (kbuchegg)]

Warum machst du das nicht so:

void CAN_PullMessage (void)
{
  unsigned char *pDst; // Destination pointer
  
  // Initialize pointers
  pDst = (unsigned int *) &(uc_RxBuf[0]);

  *(pDst++) = mRxCAN[mRxOUT].MsgID;
  *(pDst++) = mRxCAN[mRxOUT].DatA;
  *(pDst++) = mRxCAN[mRxOUT].DatB;
  *(pDst++) = mRxCAN[mRxOUT].Frame;
  
  Write_Buffer(&uc_RxBuf[0]); 
  
  // Adjust OUT pointer
   mRxOUT++;
  
   if (mRxOUT >= MAX_QUEUE)
    {
    mRxOUT = 0;
    }
}

Kein Stress mit irgendwelchen casts, der Code ist auch ohne
Kommentare verständlich und kürzer ist er auch noch.

Casts sind Waffen!
Manchmal braucht man casts. Dann sollte man aber wissen was
man tut. Bei jedem cast überlegen, ob er auch tatsächlich
notwendig ist, oder ob es nicht auch ohne geht. Denn oftmals
sind casts einfach nur der Hinweis, dass am ganzen Programmaufbau
etwas nichts stimmt.

[Nador R. (rifman)]

@Helge: Danke für die Infomationen.

@Karl heinz Buchegger: Danke für den Vorschlag hat aber leider nicht 
geklappt, der Kompiler gibt jetzt zwei warnings aus:

"cast increases required alignment of target type"
"assignment makes integer from pointer without a cast"

Ich habe trotzdem den Code heruntergeladen, es hat nicht funktioniert.

[Martin Thomas (Gast)]

Da ich schon ein wenig in meinem Quellcodearchiv gesucht hatte, hier 
mein Vorschlag. K.-H. Bucheggers Code sieht allerdings schicker aus aber 
berücksichtigt, wenn richtig gesehen, big/little-endian nicht - was aber 
im Anwendungsfalls mglw. auch garnicht erfoderlich ist.

#include <stdint.h>

// CAN_MSG typedef
// RxCan-Array, ucRxBuf, mRxOUT Definition
// MAX_QUEUE #define

// Funktion aus der efsl uebernommen:
void ex_setb32(uint8_t* buf, uint32_t offset, uint32_t data)
{
#ifdef BIG_ENDIAN
  *(buf+offset+3) = data>> 0;
  *(buf+offset+2) = data>> 8;
  *(buf+offset+1) = data>>16;
  *(buf+offset+0) = data>>24;
#else
  *(buf+offset+0) = data>> 0;
  *(buf+offset+1) = data>> 8;
  *(buf+offset+2) = data>>16;
  *(buf+offset+3) = data>>24;
#endif
}
// analog - falls erforderlich - ein "setb16"

/* serialize data from mRxCAN-Object into byte-array byte-buffer
void serialize_RxCAN( const CAN_MSG *pMessage, uint8_t *pdest ) 
{
  ex_setb32( pdest,  0, pMessage->MsgId );
  ex_setb32( pdest,  4, pMessage->DataA );
  ex_setb32( pdest,  8, pMessage->DataB );
  ex_setb32( pdest, 12, pMessage->Frame );
}

void foo() 
{
  serialize_RxCAN( &(mRxCAN[mRxOUT]), uc_RxBuf )
  Write_Buffer( uc_RxBuf ); 
  if ( mRxOUT >= MAX_QUEUE ) {
    mRxOUT = 0;
  }
}

Nur so runtergeschrieben bzw. reinkopiert - nicht getestet.

[Karl H. (kbuchegg)]

Nador Rif wrote:

> geklappt, der Kompiler gibt jetzt zwei warnings aus:
>
> "cast increases required alignment of target type"

Das ist dann wahrscheinlich hier:

  pDst = (unsigned int *) &(uc_RxBuf[0]);

Hmm. Wenn sichergestellt ist, daß uc_RxBuf so aligned
ist, dass das kein Problem darstellt, dann würde ich mal
die Compilerdoku durchforsten, wie man diese Warnung
abstellen kann.
Das Problem dieser Warnung hast du sowieso immer, egal wie
du es machst. Wenn das Alignment nicht stimmt, dann gibts
Ärger.

Ich hab übrigens einen Fehler beim Tippen gemacht.
pDst sollte ein unsigned int Pointer sein und kein
unsigned char Pointer.

Ah. Man könnte das anders rum machen, anstelle von
einem unsigned int umkopieren, könnte man sizeof(unsigned int)
Bytes umkopieren. Schneller wirds dadurch wahrscheinlich
auch nicht, schöner auch nicht.

In etwa so:

  unsigned char *pDst; // Destination pointer
  
  // Initialize pointers
  pDst = (unsigned char *) &(uc_RxBuf[0]);

  memcpy( pDst, &mRxCAN[mRxOUT].MsgID, sizeof( unsigned int ) );
  pDst += sizeof( unsigend int );
  memcpy( pDst, &mRxCAN[mRxOUT].DatA, sizeof( unsigned int ) );
  pDst += sizeof( unsigend int );
  memcpy( pDst, &mRxCAN[mRxOUT].DatB, sizeof( unsigned int ) );
  pDst += sizeof( unsigend int );
  memcpy( pDst, &mRxCAN[mRxOUT].Frame, sizeof( unsigned int ) );

  ...

(pDest ist hier wieder ein unsigned char Pointer)

> "assignment makes integer from pointer without a cast"

An welcher Stelle kommt diese Warnung?

[Karl H. (kbuchegg)]

Martin Thomas wrote:
> mein Vorschlag. K.-H. Bucheggers Code sieht allerdings schicker aus aber
> berücksichtigt, wenn richtig gesehen, big/little-endian nicht

Ist richtig. Ich hab mich an die Vorgabe des OP gehalten :-)
Ob das so passt, muss er selbst entscheiden.


Dein Code ist doch super. Besser als meiner.

[Falk B. (falk)]

@ Karl heinz Buchegger (kbuchegg) (Moderator)

>Hmm. Wenn sichergestellt ist, daß uc_RxBuf so aligned
>ist, dass das kein Problem darstellt, dann würde ich mal
>die Compilerdoku durchforsten, wie man diese Warnung
>abstellen kann.

Das würde ich lassen. Lieber die Warnung immer im Gesicht, als unter den 
tisch fallen lassen.

>du es machst. Wenn das Alignment nicht stimmt, dann gibts
>Ärger.

Nöö, warum nicht ander herum? Es geht ja anscheinend um ein Senden. 
Warum nicht mit einer ordentlichen Struktur/Variable arbeiten, die ein 
hohes Alignmet hat (2 oder 4), und DARAUF einen Pointer casten, der nur 
auf Bytes zeigt, und damit wahrscheinlich ein niedrigeres Alignment (1 
oder 2) hat).

>Ah. Man könnte das anders rum machen, anstelle von
>einem unsigned int umkopieren, könnte man sizeof(unsigned int)
>Bytes umkopieren. Schneller wirds dadurch wahrscheinlich
>auch nicht, schöner auch nicht.

Uhhhhh, jetzt wirds weiss Gott akademisch! Warum nicht einfach die 
gesammte Struktur kopieren? Man muss "nur" dafür sorgen, dass keine 
Padding Bytes drinstecken.

MFG
Falk

[Nador R. (rifman)]

@Martin Thomas:

Vielen Dank, das hat mit deinem Code geklappt.RESPEKT..!

@Karl heinz Buchegger:

Vielen Dank für deine Bemühungen.

[Nador R. (rifman)]

@Martin Thomas:
es gibt ein Problem, der Identifier trifft verkeht rum ein, das komische 
dran ist, dass alle andere Daten richtig eintreffen! und zwar data A,
data B und Frame-Daten.Als identifier sollte ich "05F7" bekommen
anstattdessen kriege ich "F705" zum Sehen! wieseo denn?

[Nador R. (rifman)]

Sorry die Frame-Daten sind auch verkehrt rum und zwar "00 04 08 00" 
anstatt "00 08 04 00".
PS:Es wird mit Little Endian gespeichert.

[Martin Thomas (Gast)]

Die ganze Umwandlerei passiert in void ex_setb32. Ich habe die Funktion 
wie geschreiben aus einem anderen Code rauskopiert (ich glaube, das 
big/little-endian entspricht darin auch nicht der üblichen Konvention - 
ist "byte-orientiert" nicht MS-Word/LS-Word). Möglicherweise muss man in 
der konrekten Anwendung uin32_t anders "auseinanderdröseln". Was heisst 
"eintreffen"? wo? Wie interpretiert der "Empfänger" die Byte-Daten? 
Evtl. muss man für Identifier und Frame jeweils eigene "serializer" 
schreiben. Wieder nur so runtergeschrieben/kopiert

void ser_id(uint8_t* buf, uint32_t offset, uint32_t data)
{
  *(buf+offset+0) = data>>16;
  *(buf+offset+1) = data>>24;
  *(buf+offset+2) = data>> 0;
  *(buf+offset+3) = data>> 8;
}

void ser_data(uint8_t* buf, uint32_t offset, uint32_t data)
{
  *(buf+offset+0) = data>> 0;
  *(buf+offset+1) = data>> 8;
  *(buf+offset+2) = data>>16;
  *(buf+offset+3) = data>>24;
}

void ser_frame(uint8_t* buf, uint32_t offset, uint32_t data)
{
  *(buf+offset+0) = data>>24;
  *(buf+offset+1) = data>>16;
  *(buf+offset+2) = data>> 8;
  *(buf+offset+3) = data>> 0;
}

void serialize_RxCAN( const CAN_MSG *pMessage, uint8_t *pdest ) 
{
  ser_id(    pdest,  0, pMessage->MsgId );
  ser_data(  pdest,  4, pMessage->DataA );
  ser_data(  pdest,  8, pMessage->DataB );
  ser_frame( pdest, 12, pMessage->Frame );
}

Passt evtl. nicht ganz richtig aber sollte ausreichend "inspirieren".

[Nador R. (rifman)]

Danke Martin, also die Can-Nachrichten werden in einem Buffer (nach 
deinem Serializer-Model) zwischengespeichert und via UART zum PC 
geschickt, ich wollte es eigentlich genau so wie du Lösung, es ist mir 
nur nicht klar warum die identifier und Frame-Daten anders als die 
Nutz-Daten im Receive-Buffer von CAN Controller eintreffen, ist das 
üblich so?

[Martin Thomas (Gast)]

Ohne zu wissen, wie das PC-Programm mit den empfangen Daten umgeht, kann 
zumindest ich dazu nichts sagen. Autor/Hersteller des PC-Programms 
fragen, in welcher Form die Daten erwartet werden (also wie deren 
"deserializer" funktioniert). Mit dem gezeigten Ansatz kann man den 
Datenstrom ja "zurechtbiegen".

[Nador R. (rifman)]

sorry, das hätte ich auch sagen müssen, ich benutze den HyperTerminal