Forum: Compiler & IDEs MISRA Regelverletzung, Grund nicht erkennbar


von Michael S. (michael8192)


Lesenswert?

Hallo zusammen,

ich muss bei meinem Programm die Misra Richtlinien einhalten und erhalte 
u.a. bei folgenden Funktionen die Regelverletzung, wobei ich nicht 
nachvollziehen kann wieso diese auftritt und was ich verändern müsste, 
um diesen Fehler zu eliminieren. Bei allen drei Fehlern wird einem uC 
Register (SFR) ein Wert zugewiesen.

Vielleicht kann mir jemand helfen?

Vielen Dank.

[ MISRA 10.1 ] The value of an expression of integer type shall not be 
implicitly converted to a different underlying type if: (a) it is not a 
conversion to a wider integer type of the same signedness, or (b) the 
expression is complex, or (c) the expression is not constant and is a 
function argument, or (d) the expression is not constant and is a return 
expression.

Funktion 1: Fehler in Zeile 5
1
uint16_type read_ADC_on_channel( uint8_type  channel )
2
{
3
  uint16_type temp;   /*Temporaere Variable zur Datenerfassung*/
4
  ADCON0 &= 0x03U;    /*ADCON0bits<7:2>=0*/
5
  ADCON0      |= ( channel<<2U );   /*HIER FEHLER, Auswahl ADC-Kanal maskieren*/
6
  ADCON0bits.GO  = 1U;  /*Start AD-Conversion*/
7
  while( ADCON0bits.GO == SET )   /*ADC busy?*/
8
  {
9
    ;
10
  }
11
  temp   = ( uint16_type )ADRESH; /*Highbyte ADC-Result nach Lowbyte temp*/
12
  temp <<= 8U;   /*Lowbyte temp nach Highbyte temp*/
13
  temp  |= ( uint16_type )ADRESL;  /*Lowbyte ADC-Result nach Lowbyte temp*/
14
  return ( temp );  /*  Rückgabe Ergebnis AD-Conversion*/
15
}

Funktion 2: Fehler in Zeile 6
1
void EEprom_write( uint16_type  addr, uint8_type  data )
2
{
3
  uint16_type addr_msk  = 0x0FFU;
4
  uint8_type  GIE_BIT_VAL = 0U;
5
  EEADR       = ( addr & addr_msk );
6
  EEDATA       = data; /*HIER FEHLER*/
7
  EECON1bits.EEPGD = 0U;
8
  EECON1bits.CFGS  = 0U;
9
  EECON1bits.WREN  = 1U;
10
  if( INTCONbits.GIE == 1U )
11
  {
12
    GIE_BIT_VAL = 1U;
13
  }
14
  else
15
  {
16
  }
17
  INTCONbits.GIE = 0U;
18
  EECON2       = 0x55U;
19
  EECON2       = 0xAAU;
20
  EECON1bits.WR  = 1U;
21
  while( EECON1bits.WR == 1U )                                   
22
  {
23
    ;
24
  }
25
  if( GIE_BIT_VAL == 1U )
26
  {
27
    INTCONbits.GIE = 1U;
28
  }
29
  else
30
  {
31
  }
32
  EECON1bits.WREN = 0U;
33
}

oder Funktion 3 ähnlich wie Funktion 1:
1
void set_pwm1_duty( uint16_type  ratio )
2
{
3
/*CCPR1L = 0x2D = 0b00101101; CCP1CON<5:4> =0b00 --> CCPR1L:CCP1CON<5:4> = 0b0010110100 = 0xB4 = 180d*/
4
  CCPR1L = 0x00U;                                           /*  set CCPR1L register to zero                                   */
5
  CCPR1L   = ( ratio>>2U );  /*HIER FEHLER, set PWM Duty cycle register high-byte*/
6
  CCP1CON &= 0xFCU;                                         /*  clear two LSbs for PWM duty (CCP1CON< 5:4 >)                         */
7
  CCP1CON |= ( uint8_type )ratio & ( uint8_type )0x03U;                       /*  set 2 LSbs to PWM Duty cycle register (CCP1CON< 5:4 >)                     */
8
}

von ARMdran (Gast)


Lesenswert?

Wie ist denn EEDATA genau definiert?

von Jörg W. (dl8dtl) (Moderator) Benutzerseite


Lesenswert?

Schreib mal:
1
  ADCON0      |= ( (unsigned int)channel<<2U );
2
...
3
  EEDATA       = (unsigned int)data;

von Bitflüsterer (Gast)


Lesenswert?

@ Jörg Wunsch

Möglich, das das geht, aber meine Vermutung ist eher, das der Suffix U 
eine Umwandlung in unsigned int des linken Operanden provoziert, der ja 
als abgeleiteter typ uint8_type in die Funktion kommt. Das hinterher 
wieder in unsigned int umzuwandeln dürfte den MISRA-Rule-Checker nicht 
beruhigen.

Handelt sich wohl um Fall a) und/oder c) handeln.

von Karl H. (kbuchegg)


Lesenswert?

Lass uns mal die MISRA Regel ein wenig analysieren
1
The value of an expression of integer type ...
es geht also um ganzzahlige Ausdrücke. Man beachte: hier wird nichts 
über signed oder unsigned ausgesagt. Die Regel gilt also gleichermassen 
für beide
1
shall not be implicitly converted to a different underlying type if:
es soll also keine implizite Typkonvertierung im Spiel sein. D.h die 
Alternative lautet: da muss es einen Cast geben, der dem Leser klar 
macht, das hier eine Typkonvertierung abläuft.

Gut. Unter welchen Umständen muss da also ein Cast hin, falls eine 
Konvertierung notwendig ist
1
(a) it is not a conversion to a wider integer type of the same signedness, or
wenn es sich um keine Konvertierung handelt, die zu einem 'größeren 
Datentyp' führt, der allerdings die gleiche Signedness haben muss (also 
signed->signed oder unsigned->unsigned).
d.h. hier ist davon die Rede, dass man einen int problemlos an einen 
long zuweisen darf. Denn ein long ist ja ein 'wider integer type' als es 
ein int darstellt.
Was ist da der Hintergrund der Sache? Na, ganz einfach. Ein long kann 
problemlos alle Werte annehmen, die ein int auch annehmen kann. Aber 
umgekehrt geht das nicht (nicht immer). Ein long kann Werte beinhalten, 
die ein int nicht annehmen kann. D.h. in diesem Fall, Zuweisung eines 
long an einen int, fordert MISRA einen expliziten Cast, damit klar 
gestellt wird, dass dieses hier beabsichtigt ist und sich der 
Programmierer überlegt hat, dass dieser long keine Werte haben kann, die 
nicht im Zahlenbereich eines int liegen.

Das gilt gleichermassen für signed und unsigned Werte, wobei eine 
Mischung nicht zulässig ist.
1
(b) the expression is complex, or
Tja. Das ist IMHO ein Gummi-Paragraf. Denn wann ist denn ein Ausdruck 
'komplex'? Was für den einen komplex ist, ist für den anderen täglich 
Brot.
1
(c) the expression is not constant and is a function argument,

Wenn es sich also nicht einfach nur um einen Zahlenwert handelt
und zusätzlich das Ergebnis davon auch noch an eine Funktion übergeben 
wird.
Letzters könnte auch so aufzufassen sein, dass Werte die eine Funktion 
über ihre Argumentliste kriegt, nicht implizit gecastet werden müssen. 
Allerdings wäre diese Interpretation IMHO ein wenig unsinnig.
1
(d) the expression is not constant and is a return expression.
Ausdrücke, die mittels return aus einer Funktion rausbugsiert werden 
müssen.


Kurz gesagt geht es darum, dass in
1
   linke_Seite = rechte_Seite
die Datentypen links und rechts nicht übereinstimmen und wie in solchen 
Situationen zu verfahren ist. Es gibt eine Ausnahme, wann ein Cast nicht 
notwendig ist ('to a wider Type') und Forderungen, wann die rechte Seite 
der Zuweisung auf den Datentyp der linken Seite gecastet werden muss.

von Karl H. (kbuchegg)


Lesenswert?

Karl Heinz schrieb:

> Kurz gesagt geht es darum, dass in
>
1
>    linke_Seite = rechte_Seite
2
>
> die Datentypen links und rechts nicht übereinstimmen und wie in solchen
> Situationen zu verfahren ist. Es gibt eine Ausnahme, wann ein Cast nicht
> notwendig ist ('to a wider Type') und Forderungen, wann die rechte Seite
> der Zuweisung auf den Datentyp der linken Seite gecastet werden muss.

Wichtig ist auch eine der Grundregeln in C:
Gerechnet wird im höchsten Datentyp, der in einer Operation vorkommt, 
mindestens aber in int.

Speziell der letzte Teilsatz ist wichtig!

Denn er besagt zb, dass hier
1
unsigned char   i;
2
3
   ... = i << 2;
der Datentyp des Ergebnisses der Shift-Operation vom Datentyp int ist 
und nicht etwa unsigned char! Selbst wenn i vom Datentyp unsigned char 
war (der Datentyp von 2 spielt bei einer Shift-Operation keine Rolle): i 
wird von unsigned char auf int erweitert, dann um 2 Stellen geshiftet 
und das Ergebnis hat den Datentyp int.

D.h. wenn man die MISRA Regeln ernst nimmt, dann gibt es hier
1
  ADCON0      |= ( channel<<2U );
(wahrscheinlich) 2 Probleme
das eine ist die Shift OPeration selbst:
channel wird von uint8_type (ich geh mal davon aus, dass das ein 
unsigned char ist), auf int erweitert.
Das steht aber in Widerspruch zu a), nach der ein Wechsel der Signedness 
ohne expliziten Cast nicht zulässig ist, selbst dann wenn der 
Zieldatentyp 'größer' als der Quelldatentyp ist, was ja bei unsigned 
char nach int eigentlich der Fall wäre.
Dann wird der Shift durchgeführt, das Ergebnis ist ein int.
Dieser int wird an ADCON0 zugewiesen. Ich geh mal davon aus, dass es 
sich dabei um eine 8-Bit unsigned Pseudo-Variable handelt. D.h. da wird 
ein int an einen unsigned char zugewiesen. Was schon mal laut Teil a) 
gar nicht ohne expliziten Cast geht.

von Jörg W. (dl8dtl) (Moderator) Benutzerseite


Lesenswert?

Bitflüsterer schrieb:
> aber meine Vermutung ist eher, das der Suffix U eine Umwandlung in
> unsigned int des linken Operanden provoziert

Das mag sich derjenige, der da "2U" hingeschrieben hat, auch gedacht
haben.

Besser wäre es gewesen, er hätte sich den Standard diesbezüglich
durchgelesen:
1
  6.5.7 Bitwise shift operators
2
3
  Semantics
4
3 The integer promotions are performed on each of the operands. The type of the result is
5
  that of the promoted left operand. […]

Steht also völlig eindeutig da.  Da die linke Seite ein "uint8_t" ist,
wird sie “promotet”.  Da alle Werte eines uint8_t in "int" passen, ist
das der daraus resultiernde Datentyp — und zwar vorzeichenbehaftet.

Das an die 2 angehängte "U" interssiert einfach nur kein Schwein hier.

Genau deshalb ist MISRA aber hier auch allergisch, denn aus dem
vorzeichenlosen Eingangswert ist jetzt implizit (das ist das
Wesentliche!) ein vorzeichenbehafteter Wert geworden.

von Bitflüsterer (Gast)


Lesenswert?

@ Karl Heinz

Interessante Interpretation. Merke doch, das ich schon ne Weile keine 
MISRA-Meldungen mehr interpretiert habe. Ist ja genauso schlimm wie 
Gesetzestexte.

Ich stimme auch an einer Stelle nicht mit Deiner Interpretation überein, 
aber, naja, warten wir mal ab, was dem TO der Vorschlag von Jörg bringt. 
Kann doch sein dass es passt.

von Karl H. (kbuchegg)


Lesenswert?

Bitflüsterer schrieb:

> Interessante Interpretation. Merke doch, das ich schon ne Weile keine
> Ich stimme auch an einer Stelle nicht mit Deiner Interpretation überein,
> aber, naja, warten wir mal ab, was dem TO der Vorschlag von Jörg bringt.
> Kann doch sein dass es passt.

Mit
1
  EEDATA       = data; /*HIER FEHLER*/
kann ich auch noch nichts anfangen.

Die Fehlermeldung in der ADC Routine, denke ich, könnte verschwinden, 
weil er ja hier
1
  ADCON0      |= ( (unsigned int)channel<<2U );
den uint8_type gezielt erst mal auf einen int (wenn auch unsigned) 
hochhebt. Ob dann bei der Zuweisung noch mal ein Downcast notwendig ist 
hängt davon ab, was sich hinter ADCON0 verbirgt.

Welche Interpretation findest du interessant? Ich muss dazu sagen, dass 
ich mit MISRA an sich nichts am Hut hab und ich einfach nur 
niedergeschrieben habe, was da im jeweiligen Satz steht, was ich an 
C-Regeln dazu noch im Kopf hab und wie sich meiner Meinung nach beides 
zueinander verhält.

Wobei ich immer noch der Meinung bin, dass ein 'zu komplex' etwas sehr 
schwammig ist, es sei denn es ist in MISRA irgendwo definieret, ab wann 
ein Ausdruck als komplex angesehen wird.

von Jörg W. (dl8dtl) (Moderator) Benutzerseite


Lesenswert?

Karl Heinz schrieb:
> Wobei ich immer noch der Meinung bin, dass ein 'zu komplex' etwas sehr
> schwammig ist, es sei denn es ist in MISRA irgendwo definieret, ab wann
> ein Ausdruck als komplex angesehen wird.

Nicht genau.  Die Intention ist aber gut erklärt, und es gibt eine
ganze Seite voller Beispiele.  Die Intention ist einfach, dass nicht
unüberschaubar Datentypen gemischt werden innerhalb eines Ausdrucks,
sondern dass klar und deutlich ist, welchen Datentyp der jeweilige
Ausdruck hat.

von Bitflüsterer (Gast)


Lesenswert?

Nanu? Heisst der Suffix 'U' nicht unsigned long int?

Aber gut.

Also wenn zwei bekannte Leute hier, die ich ziemich selten dabei ertappt 
habe das sie offensichtlichen Unsinn erzählen und von den einer noch 
beim gcc mitgewirkt hat (ich hoffe ich irre mich da gerade nicht), mir 
was erzählen, was mit meinen Überlegungen nicht übereinstimmt oder 
jedenfalls irgendwie nicht zu passen scheint, dann will ich lieber mal 
gründlich überlegen.

Jörg schrieb:

>Bitflüsterer schrieb:
>> aber meine Vermutung ist eher, das der Suffix U eine Umwandlung in
>> unsigned int des linken Operanden provoziert

>Das mag sich derjenige, der da "2U" hingeschrieben hat, auch gedacht
>haben.

Wobei ich eigentlich eher daran dachte, das der TO das "unabsichtlich" 
oder in Verkennung der tatsächlichen Vorgänge versucht hat. Jedenfalls 
aber mit der Wirkung das der linke "kleine" Operand in ein unsigned int 
umgewandelt wird, weil es der rechte schon durch das 'U' ist. (Aber 'U' 
ist ja unsigned long, also habe ich da einen Teilaspekt falsch 
beschrieben).

Das mit dem Standard ist so eine Sache. Welchen Standard? Und wer hat 
nun welches Draft oder welches tatsächlich angenommene Dokument? Welchen 
C-Stand benutzt denn der MISRA-CHecker? Ich gucke meist erstmal in den 
guten alten K&R was sachlich gesehen genauso ein Unsinn ist und ich mag 
deswegen falsch liegen.

>Das an die 2 angehängte "U" interssiert einfach nur kein Schwein hier.
Dafür finde ich nun gerade garkeine Textstelle.

> .. denn aus dem vorzeichenlosen Eingangswert ist jetzt implizit (...) >ein 
vorzeichenbehafteter Wert geworden.

Man erkennt glaube ich, das zu einem anderen Ergebnis gekommen bin. Ein 
unsigned long int der implizit kürzer gemacht wird, und je nach 
SFR-Definition wieder ein Vorzeichen kriegt.

-------------------

@ Karl Heinz B.
"Interessant" hieß hier einfach nur "gut und lesbar geschrieben".

> ... ich mit MISRA an sich nichts am Hut hab und ich einfach nur
>niedergeschrieben habe, was da im jeweiligen Satz steht, was ich an
>C-Regeln dazu noch im Kopf hab und wie sich meiner Meinung nach beides
>zueinander verhält.
Das hätte ich jetzt mal ganz frech von mir auch behauptet. Hmm. :-)

von Bitflüsterer (Gast)


Lesenswert?

@ Jörg Wunsch

Oops. Da ist mir was entglitten.

>Das mit dem Standard ist so eine Sache. ...

Ich wollte eigentlich am Ende dieses Absatzes darauf hinaus, das Du 
vielleicht mal den Standard, die Version etc. genau benennst und wenn 
möglich einen Link postest, falls Du auch ein Draft anschaust.

Aber dann ist es natürlich auch blöd das nachzuvollziehen, falls Du das 
Original hast. Man weiss ja nicht inwiefern das "allerletzte" Draft vom 
der dann angenommenen Version differiert.

von Karl H. (kbuchegg)


Lesenswert?

Bitflüsterer schrieb:
> Nanu? Heisst der Suffix 'U' nicht unsigned long int?

?
Wie kommst du da drauf.
Das U steht einfach nur für unsigned.
Ob int oder long int ist damit nicht ausgesagt.

Wie es jetzt definiert ist, nachdem die implizite int Regel ja gefallen 
ist, kann ich im genauen Wortlaut nicht sagen, aber von der Bedeutung 
her wird sich da nicht viel geändert haben. D.h. der Zahlenwert selber 
definiert durch seine Größe, ob int oder long und das U sagt einfach nur 
'unsigned'. Wenn du eine 2 explizit als unsigned long haben willst, dann 
wäre das ein "2UL"


>>Das an die 2 angehängte "U" interssiert einfach nur kein Schwein hier.
> Dafür finde ich nun gerade garkeine Textstelle.

Es folgt ganz einfach daraus, dass der Datentyp von
1
   a << b
einzig und alleine vom Datentyp von a abhängt. Der Datentyp von b ist 
dafür uninteressant. Jörg hat ja schon das entsprechende Zitat aus dem 
Standard gebracht.

Insofern unterscheidet sich
1
   a << b
gravierend von
1
   a + b

von Bitflüsterer (Gast)


Lesenswert?

@ Karl Heinz

>?
>Wie kommst du da drauf.
>Das U steht einfach nur für unsigned.
>Ob int oder long int ist damit nicht ausgesagt.

Ja. Hast recht. Mein Fehler. :-{

------
>Der Datentyp von b ist dafür uninteressant. Jörg hat ja schon das >entsprechende 
Zitat aus dem Standard gebracht.

Hm. Irgendwie hab ich heute wohl nicht den Kopf für sowas.

Bei << und >> findet zwar Integer Promotion statt aber nicht die 
"üblichen arithmetischen Umwandlungen". Deswegen deswegen gibt es keine 
Beziehung zwischen den Tpyen des linken und rechten Operanden. Deswegen 
wirkt sich der recht Operand auch nicht auf das Resultat aus.

von Michael S. (michael8192)



Lesenswert?

Guten Abend,

also ich bin hoch erfreut über die vielen Antworten. Herzlichen Dank.
Also zuerst einmal - ich habe wohl das falsche Forum gewählt. Sorry und 
danke für das Verschieben dieses Threads.
Ich habe gemerkt, dass ich einige (wohl wesentliche) Dinge nicht 
angegeben habe. Also ich verwende einen PIC18F45K22 von Microchip und 
den XC8 Compiler (Pro). Die Codezeilen
1
ADCON0      |= ( channel<<2U );   /*HIER FEHLER, Auswahl ADC-Kanal maskieren*/
2
EEDATA       = data; /*HIER FEHLER*/
3
EEDATA       = data; /*HIER FEHLER*/
beziehen sich alle auf die SFR (special function register) des PIC und 
sind alle 8bit. (ADCON0 == Controlregister ADC, CCPR1L == PWM duty cycle 
register (low byte) und EEDATA == EEPROM Datenregister).
Die hier genannten Fehler sind 3 der restlichen verbliebenen 6 Fehler 
von insgesamt über 1.500!! MISRA Fehlern. Also die meisten Fehler habe 
ich schon weggebügelt und das hat mich eine Menge Zeit und Nerven 
gekostet.

So wie im XC8 Manual angegeben, werden Konstanten (Integral Constants) 
je nach Grösse als int, long int oder long long int angelegt. Zusätzlich 
kann man mit dem Suffix "U" festlegen ob diese dann unsigned ist. Dies 
wird auch von einer der MISRA-Regeln gefordert. Also wird das "2U" vom 
Compiler als unsigned int Datentyp angelegt.
Das "rumgecaste" habe ich auch schon in mehreren Varianten ausprobiert 
mit unterschiedlichem Ergebnis: Mal keine Änderung, mal wieder Fehler 
weg und dafür ein anderer. Aber vielleicht liegt das auch an meinen 
mangelnden Kenntnissen in diesem Bereich, denn mit casts hab ich bisher 
noch nicht schaffen müssen (Weil noch nicht so oft mit MISRA zu tun 
hatte und der Compiler bisher auch ohne casts auskam und das ohne 
Probleme!)
In meiner Vorstellung muss ich ja irgendwie diesen unsigned int Typ des 
Schiebeergebnisses in das 8bit SFR des uC "hineinquetschen".
Also habe ich z.B. sowas probiert:
1
ADCON0      |= ( ((uint8_type)(channel))<<2U ); /*error, obwohl so im MISRA Manual vorgeschlagen*/
2
ADCON0      |= ( (uint8_type)channel<<2U );/*error*/
3
ADCON0      |=  (uint8_type)(channel<<2U );/*error*/
4
ADCON0      |= ( (uint8_type)channel<<(uint8_type)2U );/*ja lt. XC8 Manual kann man auch eine Konstante als 8bit unsigned definieren, trotzdem error*/
5
ADCON0      |= (uint8_type)( (uint16_type)channel<<(uint16_type)2U );/*error*/
Das gleiche Ergebnis mit
1
CCPR1L   = ( ratio>>2U );
Das Einzige was ansatzweise funktionierte war die Empfehlung des MISRA 
Manuals wenn ich nicht ADCON0 verwende sondern eine 8bit Variable, also 
in etwa so:
1
uint8_type temp = 0x00U;
2
temp |= ( ((uint8_type)(channel))<<2U ); /* Juhu Fehler weg */
3
ADCON0 = temp; /*Bingo Fehler wieder da*/
Anscheinend hat MISRA auch Probleme mit den SFRs des uC. Also bei 
misra.org gegoogelt und gefunden man soll die Hardwareregister neu 
definieren, also so:
1
/*http://www.misra.org.uk/forum/viewtopic.php?f=64&t=1029*/
2
#define TRISA ( *((volatile uint16_t*)0x1234) ) /* where 0x1234 is the address of the register */
3
...
4
5
TRISA |= 0x01; /* set bit 0 ti value 1 */
6
TRISA &= (uint8_t)~0x01; /* set bit 0 to value 0*/
Anscheinend muss der Konstantwert der Registeradresse (unsigned int, 
16bit)(konkret 0x0F92 für TRISA PIC18F45k22) einen cast haben. Aber 
TRISA als SFR hat ja 8bit.
Sinngemäss für mein ADCON0 habe ich also definiert:
1
#define ADCON0 ( *((volatile uint16_t*)0x0FC2) ) /*hat funktioniert, dafür kam eine andere Fehlermeldung bei Zuweisung:*/
2
....
3
ADCON0      |= (channel<<2U); /* irgendwas mit ich darf keinen cast verwenden*/
4
*/

Also zusammengefasst stellt sich für mich die Frage: Wie weise ich einem 
8bit Register (SFR) einen Wert zu der grösser ist als 8bit also z.B. 
unsigned int und dann noch so, dass MISRA kein Problem damit hat. Der 
Compiler scheint ja auch kein Problem zu haben wenn ich schreibe
1
ADCON0 |= (channel<<2U);

Also vielleicht kann ja jemand etwas Licht in mein Dunkel casten??
Vielen Dank.

von Jörg W. (dl8dtl) (Moderator) Benutzerseite


Lesenswert?

Bitflüsterer schrieb:
> Ich wollte eigentlich am Ende dieses Absatzes darauf hinaus, das Du
> vielleicht mal den Standard, die Version etc. genau benennst und wenn
> möglich einen Link postest, falls Du auch ein Draft anschaust.

War glaub' ich das Dokument n1336.pdf.  Allerdings unterscheiden sie
sich an der Stelle nicht, und der Vorgänger (n1124.pdf, so ich mich
nicht irre) nennt sich zwar "Draft", aber das nur der Form halber:
richtige Standards wollen/müssen sie halt verkaufen, aber es hat die
Working Group nicht daran gehindert, den tatsächlichen Standard C99
mitsamt der drei eingearbeiteten "technical corrigenda" dann nochmal
als "draft" hinzulegen.

Karl Heinz schrieb:
> Das U steht einfach nur für unsigned.
> Ob int oder long int ist damit nicht ausgesagt.

An sich schon: für "long" muss davor oder dahinter noch ein "L" stehen.

Michael S. schrieb:
> Das "rumgecaste" habe ich auch schon in mehreren Varianten ausprobiert
> mit unterschiedlichem Ergebnis:

Allerdings nicht das, was ich/wir dir vorgeschlagen haben.

Du gehst die ganze Zeit lang davon aus, dass du ein Problem auf der
linken Seite der Zuweisung hast.

Du hast aber ein Problem im Ausdruck auf der rechten Seite!  Du benutzt
dort Datentypen, die kleiner sind als "int", innerhalb von Berechnungen.
Entsprechend den Integer-Aufweitungs-Regeln werden diese dabei implizit
(und das ist das, was MISRA daran stört!) zu einem Datentyp, der
mindestens so groß wie "int" ist.  In diesem Falle ist der sich
ergebende Typ dann tatsächlich "int" und damit vorzeichenbehaftet,
obwohl dein Ausgangstyp (uint8_t) vorzeichenlos war.  Das ist das,
was MISRA daran moniert, und die Tatsache, dass du es selbst nach
intensivem Draufgucken auch noch nicht begriffen hast, zeigt, wie Recht
sie doch dabei haben.

Daher mein Vorschlag, die uint8_t-Operanden auf der rechten Seite per
Typecast auf "unsigned int" aufzuweiten (nein, nicht "uint16_t" oder
so, sondern wirklich "unsigned int").  Damit bleibt die
Vorzeichenfreiheit erhalten, die weitere Rechnung benötigt keine
integer promotion mehr.

Ob und inwiefern dann abschließend (um den ganzen Ausdruck der rechten
Seite herum) noch ein Cast auf uint8_t notwendig ist (weil das der
Zieltyp der linken Seite ist), das wird sich erst danach zeigen.  Weiß
gerade nicht, was MISRA dazu sagt; die Schwarte liegt in der Firma
herum.

von Michael S. (michael8192)


Angehängte Dateien:

Lesenswert?

Guten Morgen zusammen

Jörg Wunsch schrieb:
> Das ist das,
> was MISRA daran moniert, und die Tatsache, dass du es selbst nach
> intensivem Draufgucken auch noch nicht begriffen hast, zeigt, wie Recht
> sie doch dabei haben.
>
> Daher mein Vorschlag, die uint8_t-Operanden auf der rechten Seite per
> Typecast auf "unsigned int" aufzuweiten (nein, nicht "uint16_t" oder
> so, sondern wirklich "unsigned int").  Damit bleibt die
> Vorzeichenfreiheit erhalten, die weitere Rechnung benötigt keine
> integer promotion mehr.

Das kann durchaus sein, das ich das noch nicht ganz verstanden habe, 
sonst würde ich es ja auch richtig machen und der Fehler wäre weg :-)
Jörg Wunsch schrieb:
> Schreib mal:
>   ADCON0      |= ( (unsigned int)channel<<2U );
> ...
>   EEDATA       = (unsigned int)data;
Also wenn ich Deinen Vorschlag umsetze müsste ich das doch so machen (2U 
ist ja schon unsigned int und braucht demnach keinen Cast:
1
ADCON0        |=  ( unsigned int ) channel <<2U; /*gibt einen Fehler (10.1)*/
2
/*oder so:*/
3
ADCON0        |= ( (unsigned int )( channel ) )<<2U; /* ebenso Fehler (10.1)*/
4
/*oder so:*/
5
ADCON0        |= (( (unsigned int )( channel ) )<<2U); /* Fehler (10.1)*/
6
/*oder so?*/
7
ADCON0        |= ( (unsigned int ) channel<<2U); /* Fehler (10.1)*/
Warum sollte das eigentlich eine Rolle spielen, ob ich schreibe 
"uint16_type" oder "unsigned int"? Ich habe doch definiert:
1
typedef  unsigned  int  uint16_type;   /*Ganzzahl 16 Bit vorzeichenlos*/
Es ist sogar kontraproduktiv denn sobald ich eine Variable generell als 
"unsigned int" o.ä. deklariere, erzeuge ich mir den nächsten Fehler: [ 
MISRA 6.3 ] Typedefs that indicate size and signedness should be used in 
place of the basic types. Ist zwar "advisory" aber scheint trotzdem 
nicht ok zu sein.

Eigenartig wäre ja dann auch das, denn da ist der Fehler nämlich nach 
der Operation ((uint8_type)(channel)<<2U) weg, obwohl channel einen cast 
als 8bit unsigned hat:
Michael S. schrieb:
> Das Einzige was ansatzweise funktionierte war die Empfehlung des MISRA
> Manuals wenn ich nicht ADCON0 verwende sondern eine 8bit Variable, also
> in etwa so:uint8_type temp = 0x00U;
> temp |= ( ((uint8_type)(channel))<<2U ); /* Juhu Fehler weg */
> ADCON0 = temp; /*Bingo Fehler wieder da*/

Also nochmals getestet:
1
uint16_type read_ADC_on_channel( uint8_type  channel )
2
{
3
    uint8_type  test0 = 0x00U;
4
    uint8_type  test1 = 0x00U;
5
    uint8_type  test2 = 0x00U;
6
    uint8_type  test3 = 0x00U;
7
    uint8_type  test4 = 0x00U;
8
    uint8_type  test5 = 0x00U;
9
    uint8_type  test6 = 0x00U;
10
    uint16_type temp;                                                                              /*  < Temporaere Variable zur Datenerfassung >                                                    */
11
    ADCON0        &= 0x03U;                                                                        /*   ADCON0bits< 7:2 >=0                                                                          */
12
    test0         |= ( ( unsigned int )channel<<2U );                                              /*  Fehler 10.1 Es wird ja eine 16bit Variable einer 8bit Variable zugewiesen                     */
13
    test1         |= ( ( ( uint16_type )( channel ) )<<2U );                                       /*  Fehler 10.1 Das Gleiche egal ob unsigned int oder uint16_type                                 */
14
    test2         |= ( ( ( uint8_type )( channel ) )<<2U );                                        /*  KEIN FEHLER 10.1 , OBWOHL CAST MIT 8BIT!! SIEHE AUCH MISRA BEISPIEL IM ANHANG                 */
15
    test3         |= ( uint8_type )( ( ( ( uint16_type )( channel ) )<<2U ) );                     /*  KEIN FEHLER 10.1                                                                              */
16
    test4         |= ( uint8_type )( ( uint16_type )channel<<2U );                                 /*  KEIN FEHLER 10.1                                                                              */
17
    test5         |= ( unsigned char )( ( unsigned int )channel<<2U );                             /*  KEIN FEHLER 10.1, aber u.U. Problem mit 6.3                                                   */
18
    test6         |= ( channel<<2U );                                                              /*  AND SURPRISE SURPRISE -- > KEIN FEHLER!!!!                                                    */
19
/*ABER!!!!:*/
20
    ADCON0         = test0;                                                                        /*   FEHLER 10.1!                                                                                 */
21
    ADCON0         = test1;                                                                        /*  FEHLER 10.1!                                                                                  */
22
    ADCON0         = test2;                                                                        /*  FEHLER 10.1!                                                                                  */
23
    ADCON0         = test3;                                                                        /*  FEHLER 10.1!                                                                                  */
24
    ADCON0         = test4;                                                                        /*  FEHLER 10.1!                                                                                  */
25
    ADCON0         = test5;                                                                        /*  FEHLER 10.1!                                                                                  */
26
    ADCON0         = test6;                                                                        /*  FEHLER 10.1!                                                                                  */
27
    ADCON0bits.GO  = 1U;                                                                           /*   Start AD-Conversion                                                                          */
28
    while( ADCON0bits.GO == SET )                                                                  /*   ADC busy?                                                                                    */
29
    {
30
        ;
31
    }
32
    temp   = ( uint16_type )ADRESH;                                                                /*   Highbyte ADC-Result nach Lowbyte temp                                                        */
33
    temp <<= 8U;                                                                                   /*   Lowbyte temp nach Highbyte temp                                                              */
34
    temp  |= ( uint16_type )ADRESL;                                                                /*   Lowbyte ADC-Result nach Lowbyte temp                                                         */
35
    return ( temp );                                                                               /*   Rückgabe Ergebnis AD-Conversion                                                              */
36
}
Also die Zuweisung zu einer deklarierten unsigned char Variable (testx) 
funktioniert entweder mit 8bit cast auf channel ODER mit 16bit cast und 
anschliessendem 8bit cast ODER auch ganz OHNE CAST!! Aber die darauf 
folgende Zuweisung zum SFR ADCON0 stellt in jedem Fall ein Problem dar!!

von Bitflüsterer (Gast)


Lesenswert?

Jörg Wunsch schrieb:
> Bitflüsterer schrieb:
>> Ich wollte eigentlich am Ende dieses Absatzes darauf hinaus, das Du ...

> War glaub' ich das Dokument n1336.pdf.  Allerdings unterscheiden sie
> sich an der Stelle nicht, und der Vorgänger (n1124.pdf, so ich mich
> nicht irre) nennt sich zwar "Draft", aber das nur der Form halber:
> richtige Standards wollen/müssen sie halt verkaufen, aber es hat die
> Working Group nicht daran gehindert, den tatsächlichen Standard C99
> mitsamt der drei eingearbeiteten "technical corrigenda" dann nochmal
> als "draft" hinzulegen.

Oh. Das war mir gar nicht bekannt. Vielen herzlichen Dank, Jörg.

Wenn ich da nochmal nachhaken darf: Gibt es eine Methode oder 
irgendwelche von aussen nachzuvollziehende Anhaltspunkte, zumindest mit 
einiger Plausibilität festzustellen, wie nahe ein Draft am Standard ist? 
Ich weiss, die Formulierung ist watteweich. Aber hast Du trotzdem einen 
Hinweis?
Oder nimmst Du einfach nur den neuesten Draft und hoffst das Beste? :-)

Jedenfals sehr erhellend. Danke.

von Jörg W. (dl8dtl) (Moderator) Benutzerseite


Lesenswert?

Bitflüsterer schrieb:
> Gibt es eine Methode oder irgendwelche von aussen nachzuvollziehende
> Anhaltspunkte, zumindest mit einiger Plausibilität festzustellen, wie
> nahe ein Draft am Standard ist?

Nur darüber, was die Working Group halt dazu sagt.

http://clc-wiki.net/wiki/The_C_Standard

n1256.pdf ist die Version mit den eingearbeiteten drei TCs (mehr gab
es nicht, danach wurden C1x begonnen).

Michael S. schrieb:
> Warum sollte das eigentlich eine Rolle spielen, ob ich schreibe
> "uint16_type" oder "unsigned int"?

Weil die integer promotion rules halt von int über unsigned int über
long und unsigned long gehen.  Da man verhindern muss, dass eine
Promotion nach "int" stattfindet, hätte ich "unsigned int" genommen.

> Es ist sogar kontraproduktiv denn sobald ich eine Variable generell als
> "unsigned int" o.ä. deklariere, erzeuge ich mir den nächsten Fehler:

Stimmt auch wieder, darauf sind sie ja auch allergisch.

Aber für alles, was sie da anmosern oder nicht, habe ich auch keine
Erklärung. ;-)

von Michael S. (michael8192)


Lesenswert?

Ich habe mich mittlerweile mit folgender Lösung "angefreundet". Dadurch 
ist die ursprüngliche Fehlermeldung (10.1) weg, dafür habe ich die 
Meldung:

[ MISRA 11.3 ] A cast should not be performed between a pointer type and 
an integral type. Diese ist aber "advisory" und ich denke die kann ich 
ignorieren.
1
#define    ADCON0     ( * ( ( volatile uint16_type * ) 0x0FC2 ) )                   /*< Neudefinition Register ADCON0 wegen MISRA Regelverletzung 10.1 in Funktion           */
2
                                                   /*  read_ADC_on_channel() >                                     */
3
#define    EEDATA     ( * ( ( volatile uint16_type * ) 0x0FA8 ) )                   /*< Neudefinition Register EEDATA wegen MISRA Regelverletzung 10.1 in Funktion EEPROM_write() >   */
4
#define    CCPR1L     ( * ( ( volatile uint16_type * ) 0x0FBE ) )                   /*< Neudefinition Register CCPR1L wegen Regelverletzung 10.1 in Funktion set_pwm1_duty() >     */
Dies wurde im Misra Forum vorgeschlagen: 
http://www.misra.org.uk/forum/viewtopic.php?f=64&t=1029

in den drei Funktionen habe ich dann die Anweisungen folgendermassen:
1
uint16_type read_ADC_on_channel( uint8_type  channel )
2
{
3
  uint8_type  tmp  = 0x00U;
4
  tmp        &= 0x03U;   
5
  tmp        |= ( uint8_type )( ( uint16_type )channel<<2U ;                   
6
  ADCON0       = tmp;
7
  .....
8
}
9
10
void set_pwm1_duty( uint16_type  ratio )
11
{
12
13
  uint8_type temp = 0x00U;
14
  temp   = 0x00U;                                         
15
  temp  |= ( uint8_type )( ( uint16_type )ratio>>2U );       
16
  CCPR1L = temp;
17
  ...                
18
}
19
20
void EEprom_write( uint16_type  addr, uint8_type  data )
21
{
22
  ....
23
  EEDATA       = data;
24
  ....
25
}

Ich möchte mich hiermit nochmals recht herzlich für Eure Hilfe bedanken. 
Es hat mir doch in manchen Dingen die Augen geöffnet.

von Michael S. (michael8192)


Lesenswert?

Michael S. schrieb:
> #define    ADCON0     ( * ( ( volatile uint16_type * ) 0x0FC2 ) )
> /*< Neudefinition Register ADCON0 wegen MISRA Regelverletzung 10.1 in
> Funktion           */
>                                                    /*
> read_ADC_on_channel() >                                     */
> #define    EEDATA     ( * ( ( volatile uint16_type * ) 0x0FA8 ) )
> /*< Neudefinition Register EEDATA wegen MISRA Regelverletzung 10.1 in
> Funktion EEPROM_write() >   */
> #define    CCPR1L     ( * ( ( volatile uint16_type * ) 0x0FBE ) )
> /*< Neudefinition Register CCPR1L wegen Regelverletzung 10.1 in Funktion
> set_pwm1_duty() >     */

Sorry, ich muss natürlich für 8-Bit-Register hier schreiben:
1
#define    ADCON0     ( * ( ( volatile uint8_type * ) 0x0FC2 ) )                   /*< Neudefinition Register ADCON0 wegen MISRA Regelverletzung 10.1 in Funktion           */
2
                                                   /*  read_ADC_on_channel() >                                     */
3
#define    EEDATA     ( * ( ( volatile uint8_type * ) 0x0FA8 ) )                   /*< Neudefinition Register EEDATA wegen MISRA Regelverletzung 10.1 in Funktion EEPROM_write() >   */
4
#define    CCPR1L     ( * ( ( volatile uint8_type * ) 0x0FBE ) )                   /*< Neudefinition Register CCPR1L wegen Regelverletzung 10.1 in Funktion set_pwm1_duty() >     */

von Bitflüsterer (Gast)


Lesenswert?

@ Jörg Wunsch

Habe gerade gesehen, dass Du mir auf meine letzte Frage auch geantwortet 
hast. Vielen Dank.

Bitte melde dich an um einen Beitrag zu schreiben. Anmeldung ist kostenlos und dauert nur eine Minute.
Bestehender Account
Schon ein Account bei Google/GoogleMail? Keine Anmeldung erforderlich!
Mit Google-Account einloggen
Noch kein Account? Hier anmelden.