Re: [PATCH v2 1/2] libc: Zentralisieren der strtoxxx-Funktionen

[Kevin Wolf <kevin@xxxxxxxxxx>]

Am 14.03.2021 um 15:24 hat Nico Mayer geschrieben:
> Hallo Kevin,
> 
> vielen Dank für dein ausführliches Review.
> 
> Eine Sache verstehe ich aktuell noch nicht:
> 
> Am 13.03.21 um 00:18 schrieb Kevin Wolf:
> > > -        if (retval > (max_val - digit) / base) {
> > > -            overflow = 1;
> > > +        if (neg) {
> > > +            if (result > (min_val - digit) / base) {
> > > +                errno = ERANGE;
> > > +                overflow = 1;
> > > +            }
> > Hier wird es knifflig. Ich weiß nicht genau, was du hier checkst. Mal
> > versuchen zu analysieren, was hier passiert:
> > 
> > min_val ist unsigned long long, digit ist int. Für die Subtraktion wird
> > digit nach unsigned long long konvertiert, d.h. wir rechnen unsigned.
> > 
> > Die unsigned-Varianten lügen hier und geben ihren Maximalwert statt 0
> > als min_val. In diesem Fall tut der Code also dasselbe wie der
> > else-Block. Ich glaube, negative und positive Werte gleich zu parsen ist
> > richtig.
> > 
> > Die signed-Varianten übergeben für min_val ihr Minimum (also eine
> > negative Zahl), allerdings konvertiert zu einem unsigned long long, der
> > den Wert nicht repräsentieren kann.
> > 
> > Damit haben wir als Obergrenze für result * base + digit:
> > 
> > * für strtoll(): ULLONG_MAX - LLONG_MIN = (2^64 - 1) - 2^63 = 2^63 - 1
> > * für strtol(): ULLONG_MAX - LONG_MIN = (2^64 - 1) - 2^32
> > 
> > Ersteres ist falsch, weil -2^63 als Eingabe funktionieren sollte, aber
> > so ERANGE setzt. Alle anderen Werte sollten in Ordnung sein.
> > 
> > Zweiteres ist vollkommener Blödsinn und lässt viel zu große Eingaben zu.
> > 
> Der Codeabschnitt sollte eigentlich wie die "max_neg"-Lösung funktionieren.
> Mein zentraler Gedanke von diesem Codeabschitt war, dass das Bitmuster der
> Zahl durch die "signed->unsigned" Konvertierung" nicht geändert wird.
> 
> Daher:
> 
> (unsigned long long)LONG_MIN == 0x80000000

Das hier ist nicht richtig. Du bekommst Sign Extension und damit:

(unsigned long long) LONG_MIN == ffffffff80000000

Wenn du das ganze auf deinem x86_64-Rechner probiert hast, kriegst du
das Problem nicht, weil long da schon 64 Bit ist. Aber auf i386 geht das
schief.

Der vollständigkeit halber, im C-Standard ist das so definiert, kommt
aber aufs selbe raus:

    Otherwise, if the new type is unsigned, the value is converted by
    repeatedly adding or subtracting one more than the maximum value
    that can be represented in the new type until the value is in the
    range of the new type.60)

> (unsigned long long)LLONG_MIN == 0x8000000000000000
> 
> 
> Jetzt verstehe ich nicht ganz, wie du in deinem Review die Obergrenzen
> berechnest. Warum wird das jeweilige Minimum von ULLONG_MAX abgezogen?
> 
> Die Testfälle werde ich nochmal überprüfen. Vielleicht auch ganz cool wäre
> hier die Nutzung von Gcov. So könnte man bestimmen, wie gut getestet wird.

Meine Rechnung in der letzten Mail war off by one, aber macht es jetzt
mehr Sinn für dich?

Kevin