printf format strings prüfen?

[Bauform B. (bauformb)]

Mahlzeit!

-Wformat und besonders -Wformat-security sind ja nicht verkehrt. Ein 
nicht-konstanter format string wird dann angemeckert. Seht gut, aber es 
gibt eine Grauzone, zum Beispiel finde ich, dass diese format strings 
durchaus konstant sind. Kann ich dem gcc außer "const" noch einen Tipp 
geben?

int
my_mount (unsigned long flags, const char *source)
{
  static const char  *opttab[] = {
    "errors=remount-ro,check=strict,showexec,uid=%d,gid=%d,time_offset=%ld",
    "errors=remount-ro,uid=%d,gid=%d,time_offset=%ld",
    "errors=remount-ro",
    NULL
  };
  const char  *fsopts;
// blkid usw. findet msdos, exfat, ext4 und dann...
  fsopts = opttab[type];
  asprintf (&options, fsopts, uid, uid, timezone / 60L);
  err = mount (source, default_mnt, fstype, flags, options);

Im nächsten Beispiel ist im Endeffekt auch alles konstant, aber da ist 
die Warnung wohl unvermeidlich. Gibt es vielleicht "eine lib", mit der 
ich den format string prüfen könnte? Und wie sage ich dann dem 
strftime(), dass der ok ist?

char *
isotime (const char *format, time_t utc)
{
  static char  text[40];
  time_t       unixtime;

  if (utc == 0) {
    return (char *)"";
  }
  if (utc == 1) {
    unixtime = time (NULL);
  } else {
    unixtime = utc;
  }
  if (format == NULL || *format == '\0') {
    format = "%F %T";
  }
  strftime (text, sizeof text, format, localtime (&unixtime));
  return text;
}

Ja, natürlich kann ich das mount auch anders formulieren, da gibt es nur 
vier bekannte Dateisysteme. Aber wie man ein Datum schreibt, ist ja 
leider völlig undefiniert.

[Johann L. (gjlayde)]

Bauform B. schrieb:
> zum Beispiel finde ich, dass diese format strings durchaus
> konstant sind. Kann ich dem gcc außer "const" noch einen Tipp
> geben?

Damit ein Format-String gecheckt werden kann, muss er natürlich zur 
Compilezeit bekannt sein, und das ist

>   const char  *fsopts = opttab[type];

Nun mal nicht, weil type nicht bekannt ist zur Laufzeit.

Ich nehme mal an, dass die Obfuskation über das Array nicht super 
Performance-kritisch ist, also einfach die 3 Fälle per if+else oder 
switch+case abhandeln mit FMT-Strings die zur Compilezeit bekannt sind.

[Bauform B. (bauformb)]

Johann L. schrieb:
> Damit ein Format-String gecheckt werden kann, muss er natürlich zur
> Compilezeit bekannt sein, und das ist
>>   const char  *fsopts = opttab[type];
> Nun mal nicht, weil type nicht bekannt ist zur Laufzeit.

Ach was, natürlich ist type bekannt ;) Wenn es zu groß würde, wäre das 
ja undefined! Aber an der Stelle geht if/else problemlos, also gut.

Und jetzt auf einmal geht's für das Datumsformat genauso einfach: ich 
erlaube nur den Default oder %x "The preferred date representation for 
the current locale without the time". Wer sich ein Fantasieformat 
ausdenkt, oder in Italien die norwegische Schreibweise will, bekommt 
automatisch RFC 3339. Das muss reichen, manche Formate gehören sowieso 
verboten.

Haben wir wieder mal ein Problem elegant? umgangen. Trotzdem, so eine 
Format-Prüf-Funktion sollte es eigentlich geben, braucht man das nicht, 
wenn man gettext mit fremden Übersetzungen benutzt?

[Johann L. (gjlayde)]

Bauform B. schrieb:
> Johann L. schrieb:
>> Damit ein Format-String gecheckt werden kann, muss er natürlich zur
>> Compilezeit bekannt sein, und das ist
>>>   const char  *fsopts = opttab[type];
>> Nun mal nicht, weil type nicht bekannt ist zur Laufzeit.
>
> Ach was, natürlich ist type bekannt

Was aber nicht aus deinem Code ersichtlich ist.

[Rolf M. (rmagnus)]

Bauform B. schrieb:
> Johann L. schrieb:
>> Damit ein Format-String gecheckt werden kann, muss er natürlich zur
>> Compilezeit bekannt sein, und das ist
>>>   const char  *fsopts = opttab[type];
>> Nun mal nicht, weil type nicht bekannt ist zur Laufzeit.
>
> Ach was, natürlich ist type bekannt ;)

Wenn type schon zur Compilezeit feststeht, wozu brauchst du es dann?

[Bauform B. (bauformb)]

Ach, keiner versteht mich :( Natürlich ist das erst zur Laufzeit 
bekannt. Aber sonst heißt es immer, UB gibt es nicht. Also könnte der 
gcc sagen, es muss eins von den drei Formaten sein und die sind alle ok. 
Ich benutze allerdings auch -O, vielleicht würde er es mit -O3 sogar 
finden.

[Johann L. (gjlayde)]

Johann L. schrieb:
> Nun mal nicht, weil type nicht bekannt ist zur Laufzeit.

Mein Fehler.  Sollte natürlich heissen "nicht bekannt zur COMPILEzeit".

[Rolf M. (rmagnus)]

Bauform B. schrieb:
> Ach, keiner versteht mich :( Natürlich ist das erst zur Laufzeit
> bekannt. Aber sonst heißt es immer, UB gibt es nicht. Also könnte der
> gcc sagen, es muss eins von den drei Formaten sein und die sind alle ok.

Das könnte er möglicherweise (Wo type herkommt und wie es zu seinem Wert 
kommt, sieht man deinem Code oben nicht), aber das wäre recht komplex, 
und so ein Verhalten müsste extra da reinprogrammiert werden, für einen 
äußerst seltenen Anwendungsfall.

> Ich benutze allerdings auch -O, vielleicht würde er es mit -O3 sogar
> finden.

Höhere Optimierungsstufen sind für sowas definitv besser, weil der 
Compiler dann eine ausführliche Codeanalyse machen muss, durch die 
solche Sonderfälle eher gefunden werden können.

[Frank M. (ukw) (Moderator)]

Bauform B. schrieb:
> Natürlich ist das erst zur Laufzeit bekannt.

Da type nur die Werte 0, 1, 2 annehmen kann, mache einen switch über die 
Variable type und schreibe das printf jedesmal explizit mit dem 
konkreten Format-String hin.

Dann kannst Du Dir das array "opttab" sparen und der Code wird lesbarer. 
Außerdem kann der Compiler dann zur Compilezeit die drei Formatstrings 
prüfen.

[Bauform B. (bauformb)]

Frank M. schrieb:
> mache einen switch über die
> Variable type und schreibe das printf jedesmal explizit mit dem
> konkreten Format-String hin.

Ja, natürlich ist das in diesem Fall besser. Rein optisch sieht es jetzt 
garnicht so übel aus. Aber wenn es mehr Fälle werden, ist mir ein Liste 
mit structs (hier fstype und options) doch lieber.

static const char dosopts[] = "check=strict,showexec";

snprintf (fatopts, sizeof fatopts, "time_offset=%ld,uid=%d,gid=%d",
          timezone / 60L, uid, uid);
if (strcmp (fstype, "vfat") == 0 || strcmp (fstype, "msdos") == 0) {
   asprintf (&options, "%s,%s,%s", errors, fatopts, dosopts);
} else {
   if (strcmp (fstype, "exfat") == 0) {
      asprintf (&options, "%s,%s", errors, fatopts);
   } else {
      if (strcmp (fstype, "ext4") == 0) {
         asprintf (&options, "%s", errors);
      } else {
         fail (__func__, 0, "Unknown fstype '%s'", fstype);
      }
   }
}

[Johann L. (gjlayde)]

Bauform B. schrieb:
> Rein optisch

Viele Editoren unterstützen auch folgende Einrückung:

if (strcmp (fstype, "vfat") == 0 || strcmp (fstype, "msdos") == 0) {
   asprintf (&options, "%s,%s,%s", errors, fatopts, dosopts);
} else if (strcmp (fstype, "exfat") == 0) {
   asprintf (&options, "%s,%s", errors, fatopts);
} else if (strcmp (fstype, "ext4") == 0) {
   asprintf (&options, "%s", errors);
} else {
   fail (__func__, 0, "Unknown fstype '%s'", fstype);
}

Wobei der grauslige Stil, } anders einzurücken als das zugehörige {, 
grauslig ist.

[Frank M. (ukw) (Moderator)]

Johann L. schrieb:
> Wobei der grauslige Stil, } anders einzurücken als das zugehörige {,
> grauslig ist.

Deshalb schreibe ich die { immer auf eine extra Zeile. Dann ist rechts 
daneben auch noch jede Menge Platz für einen passenden Kommentar. :-)

P.S.

Auch für die } gebe ich mir eine extra Zeile und schreibe auch das "else 
if" darunter. Also:

if (! strcmp (fstype, "vfat") || ! strcmp (fstype, "msdos"))
{
   asprintf (&options, "%s,%s,%s", errors, fatopts, dosopts);
}
else if (! strcmp (fstype, "exfat"))
{
   asprintf (&options, "%s,%s", errors, fatopts);
}
else if (! strcmp (fstype, "ext4"))
{
   asprintf (&options, "%s", errors);
}
else
{
   fail (__func__, 0, "Unknown fstype '%s'", fstype);
}

Aber das ist Geschmackssache. Bei den heutigen Monitorgrößen muss man 
aber nicht unbedingt mit Platz geizen. Noch ein Vorteil: Ich kann mit 
dem Editor jederzeit einen else-if-Block zeilenweise rausschneiden oder 
einfügen, ohne die Formatierung in unmittelbarer Umgebung anpassen zu 
müssen.

EDIT:

strcmp-Vergleich gekürzt.

[Bauform B. (bauformb)]

Das war ja klar :) Aber so geht es natürlich auch nicht. Euch fehlen ein 
paar { { {. Entweder elif (oder so) oder die Klammern müssen sein. Spart 
ihr die { auch beim einzelnen if oder for mit nur einem Statement? Das 
ist ja nun total illegal
;)

[Daniel A. (daniel-a)]

Eventuell etwas overengineert, mit X-Macros die Structs und Funktionen 
generieren: https://godbolt.org/z/3KPKcE5e3

[Bruno V. (bruno_v)]

Bauform B. schrieb:
> Euch fehlen ein paar { { {.
Nein. Zu else if gibt es einen breiten Konsens, dass nur hier auf {} 
verzichtet werden darf, ja sogar sollte. Dafür ist das abschließende 
else Pflicht.

Die Klammern würden hier obfuskieren. Die anweisungsblöcke hingegen 
haben auch hier Klammern, egal ob 0, 1 oder mehr Anweisungen.

[Daniel A. (daniel-a)]

Bruno V. schrieb:
> Bauform B. schrieb:
>> Euch fehlen ein paar { { {.
> Nein. Zu else if gibt es einen breiten Konsens, dass nur hier auf {}
> verzichtet werden darf. Dafür ist das abschließende else Pflicht.

Ach wo, wenn es legal ist darf man es schreiben. Ich habe auch schon so 
Sachen wie "do if(){ }while();" oder "if(){}else for(;;){}" 
hingeschrieben. Das ist völlig OK.

[Oliver S. (oliverso)]

Bruno V. schrieb:
> Zu else if gibt es einen breiten Konsens, dass nur hier auf {}
> verzichtet werden darf, ja sogar sollte.

Natürlich. Genauso wie einen breiten Konsens darüber gibt, wo man 
öffnende und schließende Klammern zu setzen hat...

Immerhin werfen gcc und Clang inzwischen eine Warnung, wenn die auf else 
ohne Klammern und potentiell missverständliche Formatierungen treffen.

Klammern. Immer. Ausnahmen bestätigen die Regel.

Oliver

[Frank M. (ukw) (Moderator)]

Oliver S. schrieb:
> Immerhin werfen gcc und Clang inzwischen eine Warnung, wenn die auf else
> ohne Klammern und potentiell missverständliche Formatierungen treffen.

Das ist ja auch sinnvoll. Aber hier wird garantiert nicht gewarnt:

if (foo)
{
    bar();
}
else if (foo2)
{
    bar2();
}
else if (foo3)
{
    bar3();
}
else
{
    baz();
}

Grund: Das auf else folgende if() mit geklammerten Block dahinter ist 
für den Compiler grundsätzlich ein Statement, so dass auf die 
geschweifte Klammer zwischen else und if immer(!) verzichtet werden 
kann - wie man es vom switch() gewohnt ist. Das ist auch keine 
C-Sonderregel oder ähnliches, sondern ergibt sich klar aus der 
C-Grammatik.

Auf diese Klammer- bzw. Einrückungs-Orgie hier:

if (foo)
{
    bar();
}
else
{
    if (foo2)
    {
        bar2();
    }
    else
    {
        if (foo3)
        {
            bar3();
        }
        else
        {
            baz();
        }
    }
}

kann also immer verzichtet werden. Es reicht eine lineare Aufzählung 
aller Bedingungen. Die obere lineare Lösung ist auch wesentlich lesbarer 
als ein derart ineinander geschachtelter Block. Der wird nämlich, wenn 
eine neue Bedingung hinzukommt, immer weiter verschachtelt und nach 
rechts immer weiter eingerückt. Bei 20 Bedingungen ist man dann "ganz 
weit draußen".

[Bruno V. (bruno_v)]

Oliver S. schrieb:
> Immerhin werfen gcc und Clang inzwischen eine Warnung, wenn die auf else
> ohne Klammern und potentiell missverständliche Formatierungen treffen.

Wirklich bei else if -Reihen mit abschließendem else? (Ich nehme an, Du 
hast das ausprobiert und mit MISRA und Co bisher nichts zu tun gehabt. 
Da ist dann wohl was schiefgelaufen)

[Daniel A. (daniel-a)]

Oh Gott, jetzt auch noch MISRA. Das ist nun wirklich nur noch 
verschwendete Arbeitszeit und herausgeworfenes Geld.
Wer glaubt solche stumpfsinnige generelle Einschränkungen brächten 
irgendwas, sollte gleich Rust nehmen.