Forum: Compiler & IDEs 'var' may be uninitialized


von Michael R. (Firma: Brainit GmbH) (fisa)


Lesenswert?

Vorab: Ich weiss schon was mir die Warnung sagen will, und wie man sie 
wegkriegt, nur möchte ich das in diesem Fall vermeiden!

Es geht um einen "Treiber" für einen Sensor in C (gcc) auf AVR. Der 
Treiber hat im Wesentlichen eine init() und eine read() Funktion. im 
Init werden eine Reihe von (festen Kalibrierungs-) Paramtern vom sensor 
per i2c gelesen, mit diesen wird dann im read() das Ergebnis berechnet 
bzw. korrigiert.

Der Pseudo-Code sieht so aus:
1
uint8_t drv_init(DRIVER *Drv) 
2
{
3
   uint8_t err = i2c_blah() || i2c_write() || i2c_read(); // usw.
4
   if (err) {
5
      return err;
6
   }
7
   Drv->param1=42; // Wert wird oben per i2c gelesen
8
   Drv->param2=4711; // insgesamt 12 Parameter
9
   return 0;
10
}
11
12
uint8_t drv_read (DRIVER *Drv)
13
{
14
   int32_t val;
15
   uint8_t err = i2c_read_anything(DRV->addr, &val); // usw.
16
   if (err) {
17
      return err;
18
   }
19
   val += Drv->param1; // <== Warning
20
   val *= DRV->param2; // <== Warning
21
   return 0;
22
}

Im main wird zuerst einmal Drv_init() aufgerufen, und dann jede Sekunde 
Drv_read(). Allerdings nur wenn init() überhaupt erfolgreich war.

Nun krieg ich im read() die Warnungen dass param1, 2, ... möglicherweise 
nicht initialisiert wären.

Ursache dafür ist der "early return" im init, wenn die i2c-Kommunikation 
fehlschlägt. Grundsätzlich hat der GCC ja recht damit, und ich staune 
nicht schlecht auf welche gefinkelten Sachen der draufkommt!

Nur - in meinem Fall spielt das wenig bis keine Rolle; wenn das init 
fehlschlägt, macht das read eh keinen Sinn (und wird auch nicht 
aufgerufen, und wenn doch ist der Caller selbst schuld :-)

Eine Möglichkeit die Warnung zu beseitigen, wäre alle 12 Werte vorher 
(oder im "early return") auf 0.0 zu setzen. 12 floats zu initialisieren 
braucht aber recht viel asm-code....

Es reicht auch die 12 Werte (die eh in einer eigenen struct liegen) per 
memset() auf 0 zu setzen, interessanterweise "kneisst" der GCC sogar 
das. Resultierend sind ein paar wenige ASM-zeilen.

So richtig glücklich bin ich damit aber auch nicht...

Die Warnung generell abschalten kommt auf keinen Fall in Frage.

Kreative Ideen?

von Mark B. (markbrandis)


Lesenswert?

Alte MISRA-Weisheit: Eine Funktion braucht überhaupt nur eine 
return-Anweisung.

1
uint8_t drv_read (DRIVER *Drv)
2
{
3
    int32_t val;
4
    uint8_t retval;
5
    uint8_t err = i2c_read_anything(DRV->addr, &val); // usw.
6
7
    if (err)
8
    {
9
        retval = err;
10
    }
11
    else
12
    {
13
        val += Drv->param1;
14
        val *= DRV->param2;
15
        retval = 0;
16
    }
17
18
    return retval;
19
}

: Bearbeitet durch User
von Michael R. (Firma: Brainit GmbH) (fisa)


Lesenswert?

Mark Brandis schrieb:
> Alte MISRA-Weisheit: Eine Funktion braucht überhaupt nur eine
> return-Anweisung.

Obs eine Weisheit ist, lass ich mal dahingestellt :-) aber das ändert an 
der Warnung genau gar nix.

von Mark B. (markbrandis)


Lesenswert?

Dann erkennt der Compiler offenbar nicht, dass durch den Funktionsaufruf 
die "bewarnten" Variablen auf vernünftige Werte gesetzt werden.

Spricht etwas dagegen, dies im Fehlerfall händisch zu machen?

von Little B. (lil-b)


Lesenswert?

Ich hätte nicht erwartet, dass gcc so sensibel ist.
An dieser Stelle sehe ich keinen Grund für eine Warning, da die 
Variable, auf die zugegriffen wird, ein Funktionsparameter ist. Das kann 
dann alles mögliche sein.

Wenn du das in der Init auf 0 initialisierst, verhindert das nicht, dass 
es in der Read nicht initialisiert sein könnte.

Eine solche Warning könnte ich mir nur erklären, wenn du tiefgreifende 
Code-Checks ausführen würdest, wo die Tools dazu aber einiges kosten!
Und selbst dann dürfte keine Warning kommen, denn dann würde das tool 
die fallunterscheidung in deiner main beachten.

Vieleicht hat das aber auch was mit optimierungen zu tun, aber es wäre 
mir neu, wenn ein compiler über mehrere c-files hinweg optimieren würde.

von Torsten R. (Firma: Torrox.de) (torstenrobitzki)


Lesenswert?

Mark Brandis schrieb:
> Alte MISRA-Weisheit: Eine Funktion braucht überhaupt nur eine
> return-Anweisung.
>

Da steht doch bestimmt auch, dass man den code nicht unnötig 
verkomplizieren soll und variablen möglichst früh initialisier soll 
(würde ich da zumindest rein schreiben ;-) :
1
uint8_t drv_read (DRIVER *Drv)
2
{
3
    int32_t val = 0
4
    uint8_t retval = i2c_read_anything(DRV->addr, &val);
5
6
    if ( !retval )
7
    {
8
        val += Drv->param1;
9
        val *= DRV->param2;
10
    }
11
12
    return retval;
13
}

von Peter D. (peda)


Lesenswert?

Michael Reinelt schrieb:
> val += Drv->param1; // <== Warning
>    val *= DRV->param2; // <== Warning

Das ist schonmal ganz großer Mist.
Man unterscheidet Variablen niemals nur durch die Schreibweise. Bei 
sowas sucht man sich dumm und dämlich.

Und Warnungen immer im Klartext (copy&paste) mit Quellcode als Anhang, 
damit man die Zeilennummer zuordnen kann.

Die DRV bzw. drv werden ja wohl irgendwo angelegt werden und dann da 
einfach initialisieren. Das macht dann einmalig der Startup-Code.

von Εrnst B. (ernst)


Lesenswert?

>     int32_t val;
[...]
>     val += irgendwas;
>     val *= irgendwas_anders;


Welchen Startwert hat val?

Also:
1
     int32_t val=0x42;
2
[...]
3
     val += irgendwas;
4
     val *= irgendwas_anderes;


dass der [...]-Teil da mittels call-by-reference was reinschreiben 
könnte, ist dem GCC nicht "sicher genug".

: Bearbeitet durch User
von Torsten R. (Firma: Torrox.de) (torstenrobitzki)


Lesenswert?

Michael Reinelt schrieb:
> Nun krieg ich im read() die Warnungen dass param1, 2, ... möglicherweise
> nicht initialisiert wären.

Da Du Dich ja schon bei DRV/Drv verschrieben hast, kannst Du uns nicht 
mal den "richtigen" Code zeigen und die echte Warning? Typischer Weise 
würde der Compiler wohl eher wegen einem nicht initialisiertem "val" 
warnen.

Ansonsten: Warnungen die Buggy sind, würde ich abschalten und nicht 
drumherum programmieren und das ganze dann im Code mit einem Kommentar 
versehen.

von Torsten R. (Firma: Torrox.de) (torstenrobitzki)


Lesenswert?

Εrnst B✶ schrieb:
> Welchen Startwert hat val?

wird durch i2c_read_anything() gesetzt.

von Michael R. (Firma: Brainit GmbH) (fisa)


Lesenswert?

Mark Brandis schrieb:
> Spricht etwas dagegen, dies im Fehlerfall händisch zu machen?

Ja, wie im Ausgangspost beschrieben, ich habe keine Lust 12 floats zu 
initialisieren, und mir gefällt das memset() nicht sonderlich.

Little Basdart schrieb:
> An dieser Stelle sehe ich keinen Grund für eine Warning, da die
> Variable, auf die zugegriffen wird, ein Funktionsparameter ist. Das kann
> dann alles mögliche sein.
Doch, die Warnung ist gerechtfertigt.

> Wenn du das in der Init auf 0 initialisierst, verhindert das nicht, dass
> es in der Read nicht initialisiert sein könnte.
Doch, der GCC weiss das.

> Eine solche Warning könnte ich mir nur erklären, wenn du tiefgreifende
> Code-Checks ausführen würdest, wo die Tools dazu aber einiges kosten!
> Und selbst dann dürfte keine Warning kommen, denn dann würde das tool
> die fallunterscheidung in deiner main beachten.
Unterschätze niemals den GCC!

> Vieleicht hat das aber auch was mit optimierungen zu tun, aber es wäre
> mir neu, wenn ein compiler über mehrere c-files hinweg optimieren würde.
Such mal nach "LTO", der GCC kann das schon lange!

von Εrnst B. (ernst)


Lesenswert?

Torsten Robitzki schrieb:
> Εrnst B✶ schrieb:
>> Welchen Startwert hat val?
>
> wird durch i2c_read_anything() gesetzt.

Vermutest du. Ich hab den Quelltext dazu nicht gesehen, und der Compiler 
auch nicht (wenn er z.B. aus einer Library/seperatem Soucre-File kommt)

von Mark B. (markbrandis)


Lesenswert?

Michael Reinelt schrieb:
> Mark Brandis schrieb:
>> Spricht etwas dagegen, dies im Fehlerfall händisch zu machen?
>
> Ja, wie im Ausgangspost beschrieben, ich habe keine Lust 12 floats zu
> initialisieren

Keine Lust ist nicht wirklich ein starkes Argument. :-)

von Michael R. (Firma: Brainit GmbH) (fisa)


Angehängte Dateien:

Lesenswert?

Mark Brandis schrieb:
> Michael Reinelt schrieb:
>> Mark Brandis schrieb:
>>> Spricht etwas dagegen, dies im Fehlerfall händisch zu machen?
>>
>> Ja, wie im Ausgangspost beschrieben, ich habe keine Lust 12 floats zu
>> initialisieren
>
> Keine Lust ist nicht wirklich ein starkes Argument. :-)

Doch :-) nein, im Ernst, ich bin mir einfach nur unsicher wie die 
"saubere" Lösung aussehen könnte. memset() ist schön klein (auch im 
resultierenden asm-output) aber eher dirty. 12 Floats zu setzen wird 
(vermutlich) eher viel asm-output erzeugen (48 mov's?)

Nachdem hier keiner über Pseudo-Code nachdenken mag, häng ich den 
kompletten Code an.

SO sieht eine der Warnungen aus (insgesamt 12 Stück, alle im 
wesentlichen identisch)
1
bmp280.c:145:11: warning: ‘BMP280.calib.T1’ may be used uninitialized in this function [-Wmaybe-uninitialized]
2
     float ofs = (float) T_raw - BMP280->calib.T1;
3
           ^
4
main.c:63:14: note: ‘BMP280.calib.T1’ was declared here
5
     BMP280_t BMP280;
6
              ^

von Peter II (Gast)


Lesenswert?

wozu eigentlich der cast?
1
float ofs = (float) T_raw - BMP280->calib.T1;

von Michael R. (Firma: Brainit GmbH) (fisa)


Lesenswert?

Peter II schrieb:
> wozu eigentlich der cast?
>
>
1
> float ofs = (float) T_raw - BMP280->calib.T1;
2
>

Weil T_raw ein int ist. Du hast recht, BMP280->calib.T1 ist ein float, 
damit würde T_raw implizit gecasted, aber: hilfts nix, schadt's nix...

von Peter II (Gast)


Lesenswert?

Michael Reinelt schrieb:
> Weil T_raw ein int ist. Du hast recht, BMP280->calib.T1 ist ein float,
> damit würde T_raw implizit gecasted, aber: hilfts nix, schadt's nix...

auch wenn BMP280->calib.T1 ein ist währe, bräuchte man keinen cast.

Cast sind böse, wenn sie mal die Datentypen ändert, bekommt man fehler 
nicht mehr mit.

von Mark B. (markbrandis)


Lesenswert?

Michael Reinelt schrieb:
> 12 Floats zu setzen wird (vermutlich) eher viel asm-output erzeugen

Für nicht genutzte Rechenzeit gibt es kein Geld zurück :-)

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


Lesenswert?

Michael Reinelt schrieb:
> memset() ist schön klein (auch im resultierenden asm-output) aber eher
> dirty.

Was ist daran dirty?

Ein Bitmuster nur aus Nullen ist in IEEE 754 seit eh und je eine
eindeutige Null.  Sofern du nicht gerade auf Architekturen portabel
bleiben willst, die kein IEE 754 benutzen, ist das also absolut
kein Problem.

Was anderes würde der Startup-Code ja auch nicht tun, um irgendwelche
Gleitkommavariablen im BSS auf 0.0 zu setzen.

von Michael R. (Firma: Brainit GmbH) (fisa)


Lesenswert?

Peter II schrieb:
> Cast sind böse

Nein :-)

Peter II schrieb:
> auch wenn BMP280->calib.T1 ein int wäre, bräuchte man keinen cast.

Doch, unter Umständen schon:
1
int main (void)
2
{
3
    int a = 1<<30;
4
    int b = 1<<30;
5
    double x = a+b;
6
    double y = (double) a+b;
7
    printf ("a=%d b=%d x=%f y=%f\n", a, b, x, y);
8
}

Aber: Das hat mit dem Thema hier absolut nichts zu tun!

von Michael R. (Firma: Brainit GmbH) (fisa)


Lesenswert?

Mark Brandis schrieb:
>> 12 Floats zu setzen wird (vermutlich) eher viel asm-output erzeugen
>
> Für nicht genutzte Rechenzeit gibt es kein Geld zurück :-)

Es geht in dem Fall nicht um Rechenzeit, sodern um (asm-) Code size.

von Michael R. (Firma: Brainit GmbH) (fisa)


Lesenswert?

Jörg Wunsch schrieb:
> Was ist daran dirty?

das #include <string.h> ? :-)

> Ein Bitmuster nur aus Nullen ist in IEEE 754 seit eh und je eine
> eindeutige Null.  Sofern du nicht gerade auf Architekturen portabel
> bleiben willst, die kein IEE 754 benutzen, ist das also absolut
> kein Problem.
>
> Was anderes würde der Startup-Code ja auch nicht tun, um irgendwelche
> Gleitkommavariablen im BSS auf 0.0 zu setzen.

Ok, überzeugt. Danke!

Trotzdem - gäbs noch eine andere, schöne Alternative? Angenommen meine 
12 floats wären nicht so schön hintereinander in einer struct?

von Markus (Gast)


Lesenswert?

Wie wäre es mit folgender init():
1
uint8_t BMP280_init(BMP280_t * BMP280)
2
{
3
    uint8_t err = i2c_start_tx(BMP280->i2c_addr)
4
        || i2c_write(0xf5)      // Register 0xF5 'config'
5
        || i2c_write(BMP280->config.raw)
6
        || i2c_write(0x88)      // first calibration register
7
        || i2c_start_rx(BMP280->i2c_addr);      // repeated start for reading
8
9
    BMP280_calib C;             // calibration registers
10
    uint8_t i = sizeof(C);
11
    uint8_t *p = C.raw;
12
    while (!err && i-- > 0) {
13
        err = i2c_read(p++, i > 0);
14
    }
15
16
    if (!err) {
17
        i2c_stop();
18
    }
19
20
#if 0
21
    uart_printf("T1=%u\n", C.T1);
22
    uart_printf("T2=%d\n", C.T2);
23
    uart_printf("T3=%d\n", C.T3);
24
    uart_printf("P1=%u\n", C.P1);
25
    uart_printf("P2=%d\n", C.P2);
26
    uart_printf("P3=%d\n", C.P3);
27
    uart_printf("P4=%d\n", C.P4);
28
    uart_printf("P5=%d\n", C.P5);
29
    uart_printf("P6=%d\n", C.P6);
30
    uart_printf("P7=%d\n", C.P7);
31
    uart_printf("P8=%d\n", C.P8);
32
    uart_printf("P9=%d\n", C.P9);
33
#endif
34
35
    BMP280->calib.T1 = (float) C.T1 * pow(2, 8);
36
    BMP280->calib.T2 = (float) C.T2 * pow(2, -18);
37
    BMP280->calib.T3 = (float) C.T3 * pow(2, -42);
38
39
    BMP280->calib.P1 = (float) C.P1 * (pow(2, 8) / -100000.0);
40
    BMP280->calib.P2 = (float) C.P1 * (float) C.P2 * (pow(2, -27) / -100000.0);
41
    BMP280->calib.P3 = (float) C.P1 * (float) C.P3 * (pow(2, -47) / -100000.0);
42
43
    BMP280->calib.P4 = (float) C.P4 * pow(2, 8) - pow(2, 24);
44
    BMP280->calib.P5 = (float) C.P5 * pow(2, -10);
45
    BMP280->calib.P6 = (float) C.P6 * pow(2, -27);
46
47
    BMP280->calib.P7 = (float) C.P7 * pow(2, -4);
48
    BMP280->calib.P8 = (float) C.P8 * pow(2, -19) + 1.0;
49
    BMP280->calib.P9 = (float) C.P9 * pow(2, -35);
50
51
    if (!err) {
52
      // set power mode, start measuring
53
      err = i2c_start_tx(BMP280->i2c_addr)
54
          || i2c_write(0xf4)      // Register 0xF4 'ctrl_meas'
55
          || i2c_write(BMP280->ctrl.raw)
56
          || i2c_stop();
57
    }
58
    return err;
59
}

von Michael R. (Firma: Brainit GmbH) (fisa)


Lesenswert?

Markus schrieb:
> Wie wäre es mit folgender init():

Vermutlich dasselbe in Grün: falls die i2c-Kommunikation schon ganz am 
Anfang versagt, werden die Kalibrierungsregister nie gelesen, C.* ist 
uninitialisiert.

Abhilfe wäre auch hier ein memset() auf C

Aber ich bin eher ein Freund vom "early return" (auch wenn MISRA was 
anderes sagt...)

: Bearbeitet durch User
von Jörg W. (dl8dtl) (Moderator) Benutzerseite


Lesenswert?

Michael Reinelt schrieb:
> andere, schöne Alternative?
1
#pragma GCC diagnostic push
2
#pragma GCC diagnostic ignored "-Wuninitialized"
3
...
4
#pragma GCC diagnostic pop

;-)

von Markus (Gast)


Lesenswert?

Stimmt, dann aber wegen der variablen C.

Alternativvorschlag:
Mach die Variable BMP280 in der main zu einer globalen, dann wird sie 
vom Startup code mit Nullen initialisiert.

von Tom (Gast)


Lesenswert?

Michael Reinelt schrieb:
> Allerdings nur wenn init() überhaupt erfolgreich war.

Wo steht diese Bedingung?

von Michael R. (Firma: Brainit GmbH) (fisa)


Lesenswert?

Jörg Wunsch schrieb:
> Michael Reinelt schrieb:
>> andere, schöne Alternative?
>
>
1
> #pragma GCC diagnostic push
2
> #pragma GCC diagnostic ignored "-Wuninitialized"
3
> ...
4
> #pragma GCC diagnostic pop
5
>
>
> ;-)

Danke, danach habe ich gesucht :-)

Markus schrieb:
> Alternativvorschlag:
> Mach die Variable BMP280 in der main zu einer globalen, dann wird sie
> vom Startup code mit Nullen initialisiert.

Das ginge, ist aber unschön: Wenn man die Variable dann irgendwann doch 
wieder lokal haben möchte, hagelts plötzlich Warnungen.

Tom schrieb:
> Michael Reinelt schrieb:
>> Allerdings nur wenn init() überhaupt erfolgreich war.
>
> Wo steht diese Bedingung?

Guter Einwand!

von Falk B. (falk)


Lesenswert?

@ Michael Reinelt (fisa)

>Eine Möglichkeit die Warnung zu beseitigen, wäre alle 12 Werte vorher
>(oder im "early return") auf 0.0 zu setzen. 12 floats zu initialisieren
>braucht aber recht viel asm-code....

Ah herje. Was soll denn diese Mikrooptimierung? Intialisier den Kram 
ordentlich und gut!

von Michael R. (Firma: Brainit GmbH) (fisa)


Lesenswert?

Falk Brunner schrieb:
> Ah herje. Was soll denn diese Mikrooptimierung? Intialisier den Kram
> ordentlich und gut!

Ja eh... wird jetzt per memset() generell initialisiert, und zusätzlich 
hab ich noch einen "Verriegelung" eingebaut, dass bei fehlgeschlagener 
initialisierung das read() erst gar nicht probiert.

Danke an alle!

von Peter D. (peda)


Lesenswert?

Michael Reinelt schrieb:
> Wenn man die Variable dann irgendwann doch
> wieder lokal haben möchte, hagelts plötzlich Warnungen.

Dann mach sie eben static.

von Yalu X. (yalu) (Moderator)


Lesenswert?

Jörg Wunsch schrieb:
> #pragma GCC diagnostic ignored "-Wuninitialized"

In diesem konkreten Fall würde sogar
1
> #pragma GCC diagnostic ignored "-Wmaybe-uninitialized"

genügen. Damit wird der Compiler trotzdem noch warnen, wenn in dem
betreffenden Codeabschnitt eine Variable gelesen wird, die ganz sicher
uninitialisiert ist.

Ich persönlich hätte aber auch keine allzu großen Bedenken, die
maybe-uninitialized-Warnungen global abzuschalten, falls sie öfters zu
Fehlalarmen führen. -Wuninitialized hingegen sollte immer aktiv sein.

: Bearbeitet durch Moderator
von Peter D. (peda)


Lesenswert?

Wenn man mit float um sich schmeißt, spielt Code und Zeit eh keine 
Rolle.
Schon jede float Operation ist erheblich teurer, als ein memset.
Der AVR hat ja keine FPU, muß also alles emulieren.

: Bearbeitet durch User
von Jörg W. (dl8dtl) (Moderator) Benutzerseite


Lesenswert?

Peter Dannegger schrieb:
> Wenn man mit float um sich schmeißt, spielt Code und Zeit eh keine
> Rolle.

Peter, nur weil du keine Floats gebrauchen kannst, heißt das nicht,
dass sie generell nutzlos wären und auch nicht, dass man nicht trotzdem
versuchen sollte, den Platzbedarf zu optimieren.

Diese allgemeine Float-Allergie nervt ein wenig.

von Michael R. (Firma: Brainit GmbH) (fisa)


Lesenswert?

Peter Dannegger schrieb:
> Wenn man mit float um sich schmeißt, spielt Code und Zeit eh keine
> Rolle.

Da hab ich momentan gute Gegenargumente, eh konkret an dem Beispiel:
Beitrag "Bosch BMP280: Umrechnungen vereinfachen?"

kurz zusammengefasst:
int32:  930 Bytes Code, 2960 Takte
int64: 1366 Bytes Code, 6900 Takte
float:  578 Bytes Code, 3300 Takte

wobei die int32-Variante recht ungenau ist, und für gewisse Fälle 
(genaue Höhenmessung) damit unbrauchbar.

der float-Code hat den kleinsten footprint, und ist doppelt so schnell 
wie die int64-Variante, bei identischer Genauigkeit.

von Peter D. (peda)


Lesenswert?

Michael Reinelt schrieb:
> kurz zusammengefasst:
> int32:  930 Bytes Code, 2960 Takte
> int64: 1366 Bytes Code, 6900 Takte
> float:  578 Bytes Code, 3300 Takte

Da fehlt aber noch der Verbrauch für die Libs.
Daß int32 größer ist, liegt daran, daß oft geinlined wird, anstatt die 
Lib aufzurufen.
Daß int64 länger braucht, liegt daran, daß es auch 64Bit rechnen muß und 
nicht nur 24Bit.

von Peter D. (peda)


Lesenswert?

Jörg Wunsch schrieb:
> Diese allgemeine Float-Allergie nervt ein wenig.

Da hast Du mich gründlich mißverstanden, ich benutze auch float.
Ich meinte das nur in dem Kontext, float nehmen und sich dann über das 
popelige memset zu beschweren.

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


Lesenswert?

Peter Dannegger schrieb:
> Daß int64 länger braucht, liegt daran, daß es auch 64Bit rechnen muß und
> nicht nur 24Bit.

Das wiederum ist aber eben kein Argument für int64, sondern eins
für float.  Wenn man einen weiten Dynamikbereich abdecken muss, dann
ist Gleitkomma einfach das Mittel der Wahl.  Die 24 Bit Genauigkeit
selbst genügen ja offenbar hier vollauf, nur der Dynamikbereich von
int32 genügt nicht.

: Bearbeitet durch Moderator
von Jörg W. (dl8dtl) (Moderator) Benutzerseite


Lesenswert?

Peter Dannegger schrieb:
> float nehmen und sich dann über das popelige memset zu beschweren.

OK, er hat ja akzeptiert, dass memset() durchaus auch dafür legitim
sein kann.

Eine Funktion

void arrayset(void *array, size_t nele, union anytype initvalue);

hat der C-Standard halt vergessen zu normieren.

: Bearbeitet durch Moderator
von Michael R. (Firma: Brainit GmbH) (fisa)


Lesenswert?

Peter Dannegger schrieb:
> Ich meinte das nur in dem Kontext, float nehmen und sich dann über das
> popelige memset zu beschweren.

ich gestehe, es war neu für mich, dass memset ganz sauber zum 
initialisieren von float geeignet ist. Wieder was gelernt!

von Michael R. (Firma: Brainit GmbH) (fisa)


Lesenswert?

Jörg Wunsch schrieb:
> Die 24 Bit Genauigkeit
> selbst genügen ja offenbar hier vollauf, nur der Dynamikbereich von
> int32 genügt nicht.

Vollkommen richtig, und der Original-Code von Bosch zeigt auch sehr 
schön, wie man sich ins Knie schießen kann, wenn man verzweifelt 
versucht den Dynamikbereich doch noch irgendwie hinzumurksen, indem man 
in einem fort hin- und herschiftet.

von Peter D. (peda)


Lesenswert?

Jörg Wunsch schrieb:
> Eine Funktion
>
> void arrayset(void *array, size_t nele, union anytype initvalue);
>
> hat der C-Standard halt vergessen zu normieren.

Ich bevorzuge das Initialisieren im Startup-Code:
1
typedef union {
2
  struct{
3
    uint16_t flow_min;
4
    uint16_t temp_min;
5
    uint16_t temp_max;
6
    uint16_t press_max;
7
    uint16_t mode;
8
  }p;
9
  uint16_t v[5];
10
}ccx_wr;
11
12
ccx_wr ccx_wr_data = {
13
                        .p.flow_min  = 3000,
14
                        .p.temp_min  = 20000,
15
                        .p.press_max = 8000,
16
                        .p.temp_max  = 35000,
17
                        .p.mode = 1,
18
                     };

von Michael R. (Firma: Brainit GmbH) (fisa)


Lesenswert?

Peter Dannegger schrieb:
> Ich bevorzuge das Initialisieren im Startup-Code:

Ja, versuch ich normalerweise auch so zu machen.

In diesem Fall gehts aber um "interne" Felder (im ++-Jargon würde man 
vermutlich "private" sagen), die außerhalb des Treibers eigentlich nix 
verloren haben.

von Peter D. (peda)


Lesenswert?

Michael Reinelt schrieb:
> In diesem Fall gehts aber um "interne" Felder

Da kann man das doch auch machen.

Die Frage ist dann, ob diese Funktion mehrmals aufgerufen wird und dann 
jedesmal neu initialisiert werden muß.
Wenn nicht, dann kann man das Array static machen und es wird nur einmal 
vom Startup-Code initialisiert:
1
typedef struct{
2
    uint16_t a;
3
    uint16_t b;
4
    uint16_t c;
5
}foo;
6
7
foo * test( void )
8
{
9
  static foo fdata = {
10
                        .c = 0x7777,
11
                        .a = 0x1111,
12
                      };
13
  return &fdata;
14
}

von Michael R. (Firma: Brainit GmbH) (fisa)


Lesenswert?

Peter Dannegger schrieb:
> Die Frage ist dann, ob diese Funktion mehrmals aufgerufen wird und dann
> jedesmal neu initialisiert werden muß.
> Wenn nicht, dann kann man das Array static machen und es wird nur einmal
> vom Startup-Code initialisiert:

Nö, das geht leider nicht so einfach, da ich die ganze struct (und 
speziell diese Kalibrierungsparameter) mehrfach halten muss, da man auch 
mehrere Sensoren per i2c anschließen kann (und ich brauch in meiner 
speziellen Anwendung auch schon zwei davon)

Deswegen auch die ganze Übergabe der Struct. Sonst würd ich das eh 
Treiber-Intern machen.

Die "schöne" Variante wäre dass sich der Treiber selbst die Struktur 
allokiert und nur einen Handle zurückgibt. Hieße aber mehr oder weniger 
malloc(), und dazu hab ich am AVR keine Lust :-)

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


Lesenswert?

Michael Reinelt schrieb:
> Hieße aber mehr oder weniger malloc(), und dazu hab ich am AVR keine
> Lust :-)

Funktionieren würde es, problemlos.  Das lästigste daran dürfte der
Flash-Verbrauch für malloc() selbst sein.

von Peter D. (peda)


Lesenswert?

Michael Reinelt schrieb:
> Nö, das geht leider nicht so einfach

Warum nicht?
Der Komplexität und Anzahl der Structs sind keine Grenzen gesetzt.
Wenn die Initialisierungen für mehrere Structs gleich sind, kann man sie 
in ein Macro packen und dann das Macro entsprechend oft aufrufen.

Man kann so bequem überall Default-Werte reinschreiben und später nur 
die Unterschiede ändern.

Die einzige Einschränkung, die Anzahl der Structs muß zur Compilezeit 
bekannt sein.

von Michael R. (Firma: Brainit GmbH) (fisa)


Lesenswert?

Jörg Wunsch schrieb:
> Funktionieren würde es, problemlos.  Das lästigste daran dürfte der
> Flash-Verbrauch für malloc() selbst sein.

Ich bin bis jetzt nie in die Verlegenheit gekommen es zu brauchen. ich 
hoffe das bleibt noch eine Zeitlang so...

Peter Dannegger schrieb:
> Die einzige Einschränkung, die Anzahl der Structs muß zur Compilezeit
> bekannt sein.

Ja, darüber habe ich auch nachgedacht.
Nachdem im Regelfall eh nicht mehr als zwei angeschlossen sein können 
(es können nur zwei verschiedene i2c-Adressen per Hardware ausgewählt 
werden) wäre es vermutlich sogar am einfachsten, gleich zwei statische 
structs zu verwenden, und sogar die i2c-Adressen hart vorzugeben).

Aber für meine derzeitige Anwendung ist die momentane Variante 
ausreichend. Wenn man den Code ernsthaft veröffentlichen wollte, sollte 
man darüber nochmal nachdenken.

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


Lesenswert?

Michael Reinelt schrieb:
> Ich bin bis jetzt nie in die Verlegenheit gekommen es zu brauchen. ich
> hoffe das bleibt noch eine Zeitlang so...

Ich hab's mal (vor Jahren) geschrieben, und nachdem einige Bugs im
Laufe der Jahre gefixt worden sind und es stabil durch die Testsuite
läuft, habe ich ausreichend Vertrauen in die Implementierung. ;-)

von Michael R. (Firma: Brainit GmbH) (fisa)


Lesenswert?

Jörg Wunsch schrieb:
> Michael Reinelt schrieb:
>> Ich bin bis jetzt nie in die Verlegenheit gekommen es zu brauchen. ich
>> hoffe das bleibt noch eine Zeitlang so...
>
> Ich hab's mal (vor Jahren) geschrieben, und nachdem einige Bugs im
> Laufe der Jahre gefixt worden sind und es stabil durch die Testsuite
> läuft, habe ich ausreichend Vertrauen in die Implementierung. ;-)

Gerade durchgesehen: Schaut doch eh einfach aus :-) ist ja nicht mal ein 
buddy allocator (nein, nicht schlagen!)

Was davon braucht so viel flash?

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


Lesenswert?

Michael Reinelt schrieb:
> Was davon braucht so viel flash?

Der Code selbst halt, ein paar Zeilen sind's schon, und eben viele
Fallunterscheidungen.

malloc() und free() sind auch nicht getrennt, weil ich natürlich
davon ausgehe dass, wer malloc() benutzt, den Speicher ohnehin auch
irgendwann wieder freigeben würde. ;-)  malloc() und free() brauchen
je um die 300 Byte für ihre Implementierung.

von Daniel A. (daniel-a)


Lesenswert?

Wie funktioniert malloc auf ucs eigentlich? Woran erkennt es/was passier 
wenn der Speicher voll ist?

Ich dachte immer malloc auf ucs wäre gefährlich wegen 
Speicherfragmentierung usw.
Deshalb hab ich diese alternative geschrieben:
https://github.com/Daniel-Abrecht/DPA-UCS/blob/master/src/server/mempool.c
https://github.com/Daniel-Abrecht/DPA-UCS/blob/master/src/headers/mempool.h

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


Lesenswert?

Daniel A. schrieb:
> Wie funktioniert malloc auf ucs eigentlich?

Vermutlich nicht sehr viel anders, als es vor 40 Jahren auf einer
PDP-11 funktioniert hat.

Halt weit weniger kompliziert als sonstige heutige Algorithmen, die
vorrangig auf hohe Performance getrimmt sind auf Systemen mit
virtuellem Speicher, bei denen es auf ein paar Bytes mehr nicht ankommt.

> Woran erkennt es/

In der AVR-Implementierung: durch Vergleich gegen den Stackpointer
abzüglich einer (einstellbaren) Sicherheitsreserve, oder durch Vergleich
gegen eine vorab vorgegebene Grenze.  RTFDoc. :)

> was passier
> wenn der Speicher voll ist?

Es gibt NULL zurück, so, wie es der Standard vorsieht.

> Ich dachte immer malloc auf ucs wäre gefährlich wegen
> Speicherfragmentierung usw.

Ja, natürlich, Speicherfragmentierung ist ein gewisses Risiko, wenn
die äußeren Umstände der Anwendung es dazu bringen.  Mit anderen
Worten: du wirst immer einen pathologischen Fall konstruieren können,
an dem du nachweisen kannst, dass man den Speicher fragmentiert
bekommt.  Die Frage ist nur, ob die tatsächliche Anwendung denn
genau so in ihren Anforderungen ist.  Grob gesagt, je besser die
Anforderungen in ihrer Größe statistisch verteilt sind, um so
weniger wahrscheinlich ist eine Fragmentierung (mal vom Trivialfall
abgesehen, dass alle Requests gleich groß sind, dann fragmentiert
natürlich nie etwas).

Wenn du ein wirkliches dynamisches Problem hast (Nachrichten
vorher unbekannter Länge entstehen in mehr oder weniger zufälligen
Abständen), dann musst du so oder so das Problem lösen, und du musst
auch stets damit umgehen können, dass dir in so einer Situation der
Speicher ausgehen kann.  Damit muss eine entsprechende Applikation
irgendwie umgehen können.

Ein statisch vorbelegtes Array wird diesen Zustand nur sehr
wahrscheinlich bereits viel eher erreichen als die malloc-Variante
(dafür braucht's weniger Codegröße).  Außerdem macht es dann nicht
einmal einen Versuch, eine Überallozierung zu erkennenn (Stack
wächst in das vorbelegte Array rein, da es zu groß angelegt worden
ist).

(Wenn du kein dynamisches Problem hast, brauchst du auch kein
malloc().)

> Deshalb hab ich diese alternative geschrieben:
> https://github.com/Daniel-Abrecht/DPA-UCS/blob/master/src/server/mempool.c

Sorry, no documentation found.

Das Einzige, was man natürlich allemal besser implementieren könnte
als in malloc() wäre ein handle-basierter Allokator, der sich selbst
bei Bedarf defragmentieren kann.  Dafür wird bei einem solchen die
Applikation etwas aufwändiger, denn sie muss sich zum Handle dann
jeweils die aktuell gültige Adresse abholen.

: Bearbeitet durch Moderator
von Daniel A. (daniel-a)


Lesenswert?

Jörg Wunsch schrieb:
> Sorry, no documentation found.

Ja, das sollte ich mal erledigen. Aber wo fang ich da blos an...

> Das Einzige, was man natürlich allemal besser implementieren könnte als
> in malloc() wäre ein handle-basierter Allokator, der sich selbst bei
> Bedarf defragmentieren kann.  Dafür wird bei einem solchen die
> Applikation etwas aufwändiger, denn sie muss sich zum Handle dann
> jeweils die aktuell gültige Adresse abholen.

Das ist bei meiner Alternative der fall. Es erstellt in einem Array eine 
verkettete liste, die einen Pointer auf einen Pointer enthält, der auf 
den anfang der Daten nach den Informationen über den Listeneintrag 
zeigt. Der Allocator bekommt einen Pointer auf den Pointer dessen Wert 
dieser bei bedarf anpasst, damit er auf die Daten zeigt.

Vorteil: 1) Automatische Speicherdefragmentierung.
Nachteil:
 1) Der Pointer auf die Daten muss bis zum freigeben an der selben 
Speicherstelle gespeichert werden.
 2) Der Zugriff auf die Daten muss meistens über einen Pointer auf den 
Datenpointer erfolgen, da sich dessen wert ändern kann.

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.