Warum wird bei einem uint auf <0 getestet?

Hardy F. schrieb:
> Ich hoffe der Compiler ignoriert den Test <0

Er optimiert ihn sogar komplett weg.

Die Antwort auf die Frage der Überschrift:

 - weil der Typ erstmal zweitrangig ist und später immer wieder mal 
geändert wird.
 - weil die Warnungen aus sind
 - weil die Erfahrung fehlt

Es nützt auch nichts, das weg zu machen, bei der nächsten Änderung des 
Typs fehlt es.

Die Lösung ist also defensive Programmierung, die mit signed und 
unsigned gleichermaßen klar kommt und weder Warnung noch Overhead 
erzwingt

1

if(BC >= 127) BC=127;

2

if(BC <=   0) BC=  0;

A. S. schrieb:
> Die Antwort auf die Frage der Überschrift:
>
> weil der Typ erstmal zweitrangig ist und später immer wieder mal
> geändert wird.
> weil die Warnungen aus sind
> weil die Erfahrung fehlt
>
> Es nützt auch nichts, das weg zu machen, bei der nächsten Änderung des
> Typs fehlt es.
> Die Lösung ist also defensive Programmierung, die mit signed und
> unsigned gleichermaßen klar kommt und weder Warnung noch Overhead
> erzwingt
> if(BC >= 127) BC=127;
> if(BC <=   0) BC=  0;

Wenn wir schon dabei sind:
Wenn man das häufiger macht, gefällt mir ein Limit-Makro da besser:

1

BC = LIMIT(BC, 0, 127);

A. S. schrieb:
> Die Lösung ist also defensive Programmierung, die mit signed und
> unsigned gleichermaßen klar kommt und weder Warnung noch Overhead
> erzwingt

Das ist keine defensive Programmierung, eher das Gegenteil und natürlich 
gibt es eine Warnung (es sei denn man ist so **** und hat nicht 
mindestens -Wall -Wextra an)

Hardy F. schrieb:
> void xxx::simpleSetBrightness(uint8_t BC) {
>   if (BC > 127) {
>     BC = 127;
>   } else if (BC < 0) {
>     BC = 0;
>   }

die Funktion macht doch absolut keinen Sinn: eine lokale Variable wird 
verändert und? Das bringt doch nichts. Außerhalb sind diese Änderungen 
nicht sichtbar.

mh schrieb:
> Das ist keine defensive Programmierung, eher das Gegenteil und natürlich
> gibt es eine Warnung (es sei denn man ist so **** und hat nicht
> mindestens -Wall -Wextra an)

welche?

der Sinn? schrieb:
> die Funktion macht doch absolut keinen Sinn:

Gezeigt ist nur der Eingang. Ganz sicher wird BC dahinter irgendwem 
zugewiesen.

Magnus M. schrieb:
> jemand schrieb:
>> Wenn wir schon dabei sind:
>> Wenn man das häufiger macht, gefällt mir ein Limit-Makro da besser:BC =
>> LIMIT(BC, 0, 127);
>
> Der Vollständigkeit halber (und die Freundlichkeit gebietet es auch)
> darfst du nun noch das /#define/ nachliefern.
>
> [edit]
> ...oder steckt das in einer Bibliothek? (--> welche?)
> [/edit]

Fair, war etwas zu faul, um das auf dem Smartphone einzutippen :)

Aus einem aktuellen Projekt (welches aber bisher noch nicht getestet 
ist):

1

/** MAX-Macro halt, gibt den größeren Wert zurück. */

2

#define MAX(a, b) ((a) > (b) ? (a) : (b))

3

4

/** MIN-Macro halt, gibt den kleineren Wert zurück. */

5

#define MIN(a, b) ((a) < (b) ? (a) : (b))

6

7

/** Limitierung, die MAX() und MIN() kombiniert. */

8

#define LIMIT(low, high, a) (MIN(high, MAX(low, a)))

Da ich eigentlich immer dann auch MIN() und MAX() brauche, wenn ich so 
ein LIMIT() benutze, finde ich das so ganz praktisch und leserlicher, 
als die ternären Operatoren direkt zu schachteln.

Ist aber auch nur mein Geschmack :) .

Ansonsten bin ich bzgl. defensiver Programmierung der Meinung, dass es 
eher ein Problem ist, leichtfertig Typen quer durchs Programm zu ändern. 
Zumal das Limitieren >=0 i.A. ja sogar noch nicht einmal das gewünschte 
Verhalten sein muss, wenn man das ändert.

Aber in sicherheitsrelevantem Kram kann ich da schon einen Punkt sehen.

Mir fällt gerade auf, dass ich bei dem Beispiel oben die Argument von 
LIMIT() durcheinander gebracht habe – damit wollte ich aber auch ein 
Gegenargument gegen das Macro demonstrieren 😅 … not

Und bei dem Spezialfall aus diesem Thread könnte man es auch mit einem

1

BC &= 7f;

 lösen – bei einer Helligkeitseinstellung ist eine Bit-Limitierung 
vermutlich eh das eigentlich Ziel.

jemand schrieb:
> #define LIMIT(low, high, a) (MIN(high, MAX(low, a)))

Das macht man eigentlich nicht mehr.

 * a wird mehrfach ausgewertet. Darum heute per function Like Makros
 * Wenn man low und high verwechselt, führt es ins Chaos. (Hört sich 
hier belanglos an, erhöht den Komfort aber ungemein)

Letzteres kann man zwar mit compiletime asserts (im Makro) sichtbar 
machen, führt aber hier zu weit.

Und da der TO C++ hat, vermutlich besser Templates oder irgendwas 
abgefahrenes.

Ist aber OT, das eigentliche Problem bleibt genauso.

A. S. schrieb:
> Das macht man eigentlich nicht mehr.

Genau! Vor allem ist es auch unnötig, denn in C++ gibt es std::clamp, 
was genau das macht, aber halt in sauber (keine Mehrfach-Auswertung). 
std::min und std::max gibt es auch. MIN/MAX Makros haben in C++ 
Programmen eigentlich nichts zu suchen.

Apollo M. schrieb:
> A. S. schrieb:
>
>> a wird mehrfach ausgewertet. Darum heute per function Like Makros
>
> Kannst du das uns bitte mal zeigen?

1

#include <iostream>

2

#include <algorithm>

3

4

#define MAX(a, b) ((a) > (b) ? (a) : (b))

5

6

void test1 () {

7

  int a = 5, b = 7;

8

9

  int res = MAX(a, ++b);

10

  std::cout << "test1: " << res << std::endl;

11

}

12

13

void test2 () {

14

  int a = 5, b = 7;

15

16

  int res = std::max (a, ++b);

17

  std::cout << "test2: " << res << std::endl;

18

}

19

20

int main() {

21

  test1 ();

22

  test2 ();

23

  return 0;

24

}

https://ideone.com/rZZPMI

Der Code von test1 sieht so aus, als wäre die Ausgabe 7, d.h. als würde 
b nur einmal inkrementiert. Tatsächlich wird es aber 2x inkrementiert, 
d.h. die Ausgabe ist 9. Das ist zwar nicht falsch, aber mindestens mal 
unerwartet und schlecht lesbar. test2 mit std::max ist da deutlich 
intuitiver, es wird 8 ausgegeben. In C mit Makros kann man das nicht 
lösen, in C++ halt schon mit templates (std::max, std::min, std::clamp 
sind welche). Das mit "function like macros" ist Quatsch - die gezeigten 
MIN/MAX/LIMIT-Makros sind function-like, lösen aber das Problem nicht. 
Im GCC kann man das mit Statement Expressions machen, aber das ist halt 
nicht portabel und auch nicht besonders schön. Und wenn man einen GCC 
zur Verfügung hat, hat man auch einen G++, der C++ und templates kann, 
wie eben bei Arduino.

jemand schrieb:
> Und bei dem Spezialfall aus diesem Thread könnte man es auch mit einemBC
> &= 7f;
>  lösen

Nein.
Das macht etwas ganz anderes und kommt zu anderen Ergebnissen.

Apollo M. schrieb:
> Programmierer schrieb:
>> Das ist zwar nicht falsch, aber mindestens mal
>> unerwartet und schlecht lesbar.
>
> Gnu https://gcc.gnu.org/onlinedocs/gcc-3.1.1/gcc/Statement-Exprs.html
> zeigt dazu eine Lösung,
>
> #define maxint(a,b) ({int _a = (a), _b = (b); _a > _b ? _a : _b; })

Das ist unerwartet und schlecht lesbar. Unerwartet, weil das Verhalten 
für ein Makro in c oder c++ eigentlich nicht möglich ist. Und schlecht 
lesbar, weil es kein c oder c++ ist.

mh schrieb:
> Und schlecht lesbar, weil es kein c oder c++ ist.

Und es funktioniert nur mit int - da könnte man auch einfach eine ganz 
normale Funktion nehmen! Der Sinn des MIN/MAX-Makros bzw. der 
std::min/max Funktionen ist ja, dass sie mit allen Typen funktionieren, 
die einen kleiner-Operator haben.

der Sinn? schrieb:
> die Funktion macht doch absolut keinen Sinn: eine lokale Variable wird
> verändert und? Das bringt doch nichts. Außerhalb sind diese Änderungen
> nicht sichtbar.
Wozu müssen die außerhalb sichbar sein? Der gepostete Code ist ja nur 
ein Auszug, der das für die Frage Wesentliche darstellt. Die Funktion 
wird mit dem BC schon noch was machen, was aber für die Frage 
uninteressant ist.
Zähle einfach mal die { und die }, dann merkste schon das der gepostete 
Code nich alles ist.

Programmierer schrieb:
> mh schrieb:
>> Und schlecht lesbar, weil es kein c oder c++ ist.
>
> Und es funktioniert nur mit int - da könnte man auch einfach eine ganz
> normale Funktion nehmen! Der Sinn des MIN/MAX-Makros bzw. der
> std::min/max Funktionen ist ja, dass sie mit allen Typen funktionieren,
> die einen kleiner-Operator haben.

Da kann man sich in C schnell Abhilfe schaffen mit "generic selection". 
Damit kann man dann standardkonform z.B. sowas wie

1

#define clip(val, min_val, max_val) gen_min(gen_max(min_val, val), max_val)

nutzen, wenn gen_max und gen_min entsprechende generic selection macros 
sind.

c-hater schrieb:
> jemand schrieb:
>
>> Und bei dem Spezialfall aus diesem Thread könnte man es auch mit einemBC
>> &= 7f;
>> lösen
>
> Nein.
> Das macht etwas ganz anderes und kommt zu anderen Ergebnissen.

Ups, klar, da war ich etwas umnachtet.

A. S. schrieb:
> jemand schrieb:
>
>> #define LIMIT(low, high, a) (MIN(high, MAX(low, a)))
>
> Das macht man eigentlich nicht mehr.
>
> a wird mehrfach ausgewertet. Darum heute per function Like Makros
> Wenn man low und high verwechselt, führt es ins Chaos. (Hört sich hier
> belanglos an, erhöht den Komfort aber ungemein)
>
> Letzteres kann man zwar mit compiletime asserts (im Makro) sichtbar
> machen, führt aber hier zu weit.
> Und da der TO C++ hat, vermutlich besser Templates oder irgendwas
> abgefahrenes.
> Ist aber OT, das eigentliche Problem bleibt genauso.

Ich vergesse immer, dass Arduino ja C++ ist. Aber ja, clamp, min, max 
gibt es da ja schon alles und afaik keinen Grund, das nicht zu benutzen 
(wenn die Unterstützung es hergibt).

In C gibt es ja sonst noch _Generic, das in Kombination mit einem Haufen 
Inline Funktionen ist vielleicht auch die bessere Lösung. Vielleicht 
mache ich das in meiner Codebasis sogar tatsächlich.

Programmierer schrieb:
> Das mit "function like macros" ist Quatsch - die gezeigten
> MIN/MAX/LIMIT-Makros sind function-like, lösen aber das Problem nicht.

Ja, sorry. Was ich meinte war mit "_Generic", was seit C11 oder so geht. 
Zu unleserlich für ein Beispiel am Handy.

Hardy F. schrieb:
> Und bei uint werden durch die unterdrückten Warnungen nicht zu viel
> Protokolliert und der unnütze Code wird nicht erzeugt.

Das ist aber Harakiri. Schalt die Warnungen ein und ändere die Abfrage 
wie gezeigt auf <= bzw. >= oder mit C++ clamp. Dann hast Du Ruhe und 
Warnungen wo du sie brauchst.

Der gezeigte Code ist schlecht und sollte verbessert werden.

jemand schrieb:
> Wenn wir schon dabei sind:
> Wenn man das häufiger macht, gefällt mir ein Limit-Makro da besser:BC =
> LIMIT(BC, 0, 127);

Warum bloß immer wieder so etwas aufbauschen?
Ein

1

 if BC > 127 then BC:= 127;

2

 ...

reicht doch völlig aus.
Und normalerweise gilt, daß eine Prozedur oder Funktion den Typ ihres 
Argumentes vorgibt und sich die aufrufenden Programmteile daran halten 
müssen. Da ist nix mit "weil der Typ erstmal zweitrangig ist", sondern 
der Typ des Argumentes ist in der Funktion definiert und zwar 'unsigned 
char'.

W.S.

W.S. schrieb:
> Warum bloß immer wieder so etwas aufbauschen?

Dein if-Konstrukt ist aber noch länger. Die Verwendung von min/max/clamp 
ist deutlich lesbarer und auch an einem funktionalen Stil orientiert und 
daher sauberer. Und Pascal ist sowieso völlig verkehrt hier.

W.S. schrieb:
> Und normalerweise gilt, daß eine Prozedur oder Funktion den Typ ihres
> Argumentes vorgibt und sich die aufrufenden Programmteile daran halten
> müssen.

Du hast es überhaupt nicht verstanden. Es geht darum, den Fall 
abzufangen, wenn man den Typ in der Signatur ändert. Ich finde es auch 
etwas unnötig aber nicht falsch. Wichtiger wird das in templates. Die 
Warnung sollte man natürlich einschalten.

jemand schrieb:
> In C gibt es ja sonst noch _Generic, das in Kombination mit einem Haufen
> Inline Funktionen ist vielleicht auch die bessere Lösung.

Das ist aber eine Menge Code-Duplikation, auch nicht die feine englische 
Art. Wenn, dann sollte man einen Zeiger zurückgeben und den im Makro 
dereferenzieren, sodass praktisch ein lvalue zurückgegeben wird, von dem 
man auch die Adresse nehmen kann, genau wie bei std::min/max/clamp.

W.S. schrieb:
> Ein if BC > 127 then BC:= 127;
>  ...
> reicht doch völlig aus.

Im OP geht alles (ohne Performance-Verlust):

 - nur der Test auf >127 da unsigned
 - Makros wie MIN/MAX da hier kein ++ oder so mit drin ist
 - der untere Test mit <= statt <

Alles 3 führt aber (in anderem Kontext) zu Problemen oder Warnungen, 
wenn jemand
 - den Typ auf signed ändert
 - ein ++ oder einen Funktionsaufruf in die Afrage packt
 - der Typ mit volatile qualifiziert wird.

Von daher gibt es keine allgemeingültige Lösung für das Problem. Und 
dass eine Variable oder Argument "BC" im Laufe des Projekts den Typ 
ändert, ist nur dann selten, wenn der Code schlecht ist und deshalb 
nicht mehr geändert wird.

Hardy F. schrieb:

>

1

> void xxx::simpleSetBrightness(uint8_t BC) {

2

>   if (BC > 127) {

3

>     BC = 127;

4

>   } else if (BC < 0) {

5

>     BC = 0;

6

>   }

7

>

Das ist halt Murks. Hier kommt es her:

https://github.com/adafruit/Adafruit_TLC59711/commit/c9df7f3bad166c6b92a94633c13eb44aed442d40

Apollo M. schrieb:
> Programmierer schrieb:
>> Und es funktioniert nur mit int
>
> eher nöh!
>
> weil nach
> https://gcc.gnu.org/onlinedocs/gcc-3.1.1/gcc/Naming-Types.html#Naming%20Types
>
> es auch für beliebige types (auch expression) geht
>
> #define max(a,b) \
>   ({typedef _ta = (a), _tb = (b);  \
>    _ta _a = (a); _tb _b = (b);     \
>    _a > _b ? _a : _b; })

Wird für

1

typedef _ta = (a);

der Ausdruck (a) ausgewertet (also mit Seiteneffekten und co), oder wird 
nur der Typ bestimmt? Das wird aus dem Link nicht klar.

A. S. schrieb:
> wenn jemand
>  - den Typ auf signed ändert

Wenn jemand in den Innereien eines Objekts herumändert, dann muß er auch 
darauf achten, dort nicht wie der Elefant im Porzellanladen 
herumzutrampeln.

Wenn er dennoch herum trampelt, dann hilft dagegen garnix.

W.S.

W.S. schrieb:
> Wenn jemand in den Innereien eines Objekts herumändert,

lies doch bitte den Original-Post und die Frage.

Rolf M. schrieb:
> Doch, das bereits erwähnte clamp-Template (ob nun das Standard-Template
> oder ein selbstgebasteltes), da es hier um C++ geht.

Klar. Hab ich ja extra geschrieben. Nur verlagert es das eigentliche 
Problem (eine Abfrage auf > bzw. < am Wertebereich-Rand) auf die 
Implementierung von diesem Template.

Apollo M. schrieb:
> mh schrieb:
>> der Ausdruck (a) ausgewertet (also mit Seiteneffekten und co), oder wird
>> nur der Typ bestimmt? Das wird aus dem Link nicht klar.
>
> Wenn ich es richtig rate/verstehe, dann wird nur der type des ergebnis
> des Ausdruckes bestimmt, dazu werden dann nur die types eines "beliebig
> komplexen" Ausdrucks untersucht ... alles auch nur Glaskugel. Aber ich
> denke, dass es eher keine side effects mehr gibt.

Genau das was man in einem Programm haben möchte, ein schlecht 
dokumentiertes, compilerspezifisches Feature. Wie findet man 
Informationen zu diesem Kontrukt, wenn man es nicht kennt und darüber 
stolpert? Google findet bei mir nichts dazu.

Rolf M. schrieb:
> Dort steht übrigens auch was zur Auswertung:
> "The operand of typeof is evaluated for its side effects if and only if
> it is an expression of variably modified type or the name of such a
> type."
> Es scheint auch als Compiler-Erweiterung verbreiteter zu sein.

Weißt du zufällig was es mit "name of such a type" auf sich hat? Was und 
wie wird da "evaluated for its side effects"?

Ist es wirklich besser if-else durch gcc-Zeug zu ersetzen?

nein schrieb:
> Ist es wirklich besser if-else durch gcc-Zeug zu ersetzen?

natürlich nicht.

Hardy F. schrieb:
> void xxx::simpleSetBrightness(uint8_t BC) {
>   if (BC > 127) {
>     BC = 127;
>   } else if (BC < 0) {
>     BC = 0;
>   }

Hardy F. schrieb:
> Wenn ich das alles richtig gelesen
> habe ist es nicht falsch, falls der Datentyp geändert wird.

Aus meiner Sicht ist es falsch.

Ich habe irgendeine Bitsequenz. Von 0000.0000 bis 0111.1111 werden die 
Werte (7 Bits) übernommen. Ab 1000.0000 werden die unsigned Werte auf 
127 gesetzt (maximale brightness).

Wird diese Sequenz aber als signed betrachtet (int8_t),  dann ist dieser 
Wert <0  und wird auf 0 gesetzt (minimale brigthness).

Die gleiche Bitsequenz wird unterschiedlich behandetl und erzeugt 
verschiedene "brightness".

Rolf M. schrieb:
> Es löst die von dir genannten Probleme, ohne dabei irgendwelche
> Compiler-Eigenheiten zu nutzen.

Na eben nicht.

Das Problem: was mache ich, wenn ein Test auf < 0 überflüssig wird, weil 
der Typ unsigned ist. Bzw. allgemein: Wenn die zu testende Grenze die 
des Wertebereichs ist.

Das in eine Funktion auszulagern und es dort zu ignorieren, ist ... 
ignorant, nicht smart.

A. S. schrieb:
> Das Problem: was mache ich, wenn ein Test auf < 0 überflüssig wird, weil
> der Typ unsigned ist. Bzw. allgemein: Wenn die zu testende Grenze die
> des Wertebereichs ist.

Man könnte so etwas machen:

1

void xxx::simpleSetBrightness(uint8_t BC) {

2

  if constexpr (std::is_signed_v<std::remove_cv<std::remove_reference_t<decltype(BC)>>>) {

3

    if (BC < 0) {

4

      BC = 0;

5

    }

6

  }

7

...

aber an irgend einem Punkt wird's halt ein bisschen albern.

A. S. schrieb:
> Rolf M. schrieb:
>> Es löst die von dir genannten Probleme, ohne dabei irgendwelche
>> Compiler-Eigenheiten zu nutzen.
>
> Na eben nicht.
>
> Das Problem: was mache ich, wenn ein Test auf < 0 überflüssig wird, weil
> der Typ unsigned ist. Bzw. allgemein: Wenn die zu testende Grenze die
> des Wertebereichs ist.
>
> Das in eine Funktion auszulagern und es dort zu ignorieren, ist ...
> ignorant, nicht smart.

Was genau ist das Problem, das du dabei siehst? Und was genau möchtest 
du eigentlich als Ziel haben?

jemand schrieb:
> Ansonsten bin ich bzgl. defensiver Programmierung der Meinung, dass es
> eher ein Problem ist, leichtfertig Typen quer durchs Programm zu ändern.

Volle Zustimmung. Da hat jemand das Design verka**t.
An einem festgelegten Interface wird nicht einfach geändert.

Datentyp ändern ist wie Zweck einer Variablen ändern.
Die Auswirkungen müssen geprüft werden.

Die Diskussionen um MIN,MAC,LIMIT sind sinnvoll, lenken aber vom Thema 
ab.

Klaus schrieb:
> An einem festgelegten Interface wird nicht einfach geändert.

Was wenn es keine extern sichtbare Funktion wäre?

Programmierer schrieb:
> Klaus schrieb:
>> An einem festgelegten Interface wird nicht einfach geändert.
> Was wenn es keine extern sichtbare Funktion wäre?

Beantworte es selber: hast du es intern mit mehreren Entwicklern 
genutzt?
Hast nur Du die Funktion genutzt? Kannst Du auswendig sagen an welchen 
Stellen sich die Änderung auswirkt?

Es gibt kein gut und böse.

Es gibt nur: wie wirkt sich die Änderung aus?

Und wenn 50 Entwickler mit einer nicht extern sichtbaren Funktion 
arbeiten ist das ggf wichtiger als wenn ein externer Entwickler an einer 
sichtbaren Funktion arbeitet....

Carsten P. schrieb:
> Wer meint, dass man mit der
> Arduino-IDE kommerziell brauchbare Produkte/Software bauen kann, der
> irrt.

weil?