Prometheus EP: _total metrics malformed since 0.8.63

[fsck-block]

Seit dem code review commit in Version 0.0.63 sind im Prometheus EP leider die _total Metriken falsch.

Kommt durch das ersetzen von strncpy(total, "_total", sizeof(total)); durch strncpy(total, "_total", 6); in der Methode showMetrics
Der Compiler kann wohl doch besser zählen, denn total ist deklariert als char total[7]; 😆 😆 😆

Ich stelle noch einen kleinen PR zur Verfügung

[lumapu]

fast, es gibt da noch den berühmten Null-Terminator, der auch ein Byte belegt. Daher hat man nur effektive Nutzdaten von sizeof(total)-1.
Ich habe es versucht in 0.8.75 besser zu lösen

[lumapu]

sizeof(total)-1 ist nichtmal 'teurer':

#include <stdio.h>

int main() {

    char total[7];

    snprintf(total, sizeof(total) -1, "_total");
    snprintf(total, 6, "_total");


    return 0;
}

und hier das Assembly

.LC0:
        .string "_total"
main:
        push    rbp
        mov     rbp, rsp
        sub     rsp, 16
        lea     rax, [rbp-7]
        mov     edx, OFFSET FLAT:.LC0
        mov     esi, 6
        mov     rdi, rax
        mov     eax, 0
        call    snprintf
        lea     rax, [rbp-7]
        mov     edx, OFFSET FLAT:.LC0
        mov     esi, 6
        mov     rdi, rax
        mov     eax, 0
        call    snprintf
        mov     eax, 0
        leave
        ret

https://godbolt.org/z/xjnhE9War

[fsck-block]

Hallo @lumapu

fast, es gibt da noch den berühmten Null-Terminator, der auch ein Byte belegt. Daher hat man nur effektive Nutzdaten von sizeof(total)-1.

Da möchte ich dir widersprechen. Der Null-Terminator muss an dieser Stelle nicht abgezogen werden.

The snprintf() function is similar to printf(), but writes its output as a string in the buffer referenced by the first pointer argument, dest, rather than to stdout. Furthermore, the second argument, n, specifies the maximum number of characters that snprintf() may write to the buffer, including the terminating null character. If n is too small to accommodate the complete output string, then snprintf() writes only the first n -1 characters of the output, followed by a null character, and discards the rest.

Es ist garantiert dass nur n-1 Zeichen und ein Null-byte im Puffer landen.
Also ist

    char puffer[7];
    snprintf(puffer, sizeof(puffer) , "_total"); // _total sind 6 Zeichen

aus meiner Sicht genau richtig (und im diesem Fall auch notwendig).
https://godbolt.org/z/7cWa1K1n7

Für mich ist das Elegante daran dass ich mit snprintf (puffer, sizeof(puffer), ... selbst nichts mehr rechnen muss und damit weniger Fehler machen kann.

BTW: So wie es jetzt implementiert ist funktioniert's auch nicht, da die Zeichenkette _total zu _tota verkürzt wird und die Metrik angabe wieder nicht passt. Ich müsste also den Puffer um ein Byte vergrößern.

Soll ich den PR nochmals anfassen?

[lumapu]

ok, verstehe. Nein brauchst du nicht. Dann ist snprintf schlauer als ich gedacht habe 😉

[fsck-block]

Leider ist mir eine dem Code Review zum Opfer gefallene Zeile im PR durchgerutscht.
In der 0.8.76 fehlt in Zeile 786 / web.h nach der deklaration der Variablen total noch initialisieren derselben.

  char total[7];
  total[0] = 0;

Soll ich einen neuen Commit auf den bestehenden PR #1411 machen oder einen neuen PR erstellen?

[lumapu]

sofern ich es im kommenden commit (hoffentlich heute) vergesse dann ja 😉

[fsck-block]

Danke für's einbauen.

Da du vorgestern mal das Thema 'teuer' vs. 'billig' erwähnt hast, habe ich mit dem neuen Code mal etwas auf https://godbolt.org/ herumgespielt.

Hier das Ergebnis:

Interessant zu sehen und mit der Seite auch relativ leicht zu machen.
Aber ich glaube nicht dass man an dieser Stelle Optimierungsaufwand rein stecken sollte.

Nochmals Danke für's aufnehmen.

[lumapu]

finde interessant, dass der Xtensa ESP32 gcc bei der Zeile char puffer[7] = {'a'}; ein anderes Ergebis liefert als der x86 gcc.
Ich kenne es so, dass bei char puffer[7] = {'a'}; nur das erste byte mit 'a' initialisiert wird, der Rest mit 0. Beim ESP32 gcc sind wohl alle Bytes dann 'a'.

PS: der Compiler Explorer ist wahnsinnig interresant um solche Dinge besser zu verstehen.