Compiler warning: operation on ‘x’ may be undefined

[Jürgen W. (lovos)]

Hallo,
ich habe folgenden code:

  char line[0x100];
  char *s1=NULL;
  int noMan;
  ...
  if (strncmp(line,s1="Number=",strlen(s1))==0) {
    noMan = atoi(line+strlen(s1));
  }
  else if (strncmp(line,s1="Value=",strlen(s1))==0) {...}
  else if (strncmp(line,s1="Pos=",strlen(s1))==0) {...}

Der zwar funktioniert, aber der Compiler (gcc) erzeugt die Warnung:

warning: operation on ‘s1’ may be undefined

Warum eigentlich?
Vor dem Aufruf einer Funktion werden die Argumente von links nach rechts 
berechnet. Beim Zweiten Argument ist 's1' bekannt und wird beim 3. 
Argument verwendet.
Die Schreibweise

  if (strncmp(line,"Number=",7)==0) {}

halte ich für zu riskant.
Was anderes fällt mir nicht ein, um einen String in einer else-if-Chain 
auf verschiedene Tokens zu überprüfen.

[Peter II (Gast)]

Jürgen G. schrieb:
> Vor dem Aufruf einer Funktion werden die Argumente von links nach rechts
> berechnet.
wer sagt das?

Man könnte mit defines arbeiten:

#define _STR_NUMBER "Number="

if (strncmp(line,_STR_NUMBER ,strlen(_STR_NUMBER))==0) {
  noMan = atoi(line+strlen(_STR_NUMBER));
}

dürfte auch noch schneller sein, weil der compiler schon strlen 
ausrechnen kann.

[Jürgen W. (lovos)]

>> Vor dem Aufruf einer Funktion werden die Argumente von links nach rechts
>> berechnet.
>wer sagt das?

In C ist (fast) alles definiert bzgl. Auswertung von Ausdrücken.

Aber das scheint bei "arguments to functions" nicht der Fall zu sein.

http://en.wikipedia.org/wiki/C_%28programming_language%29
Expressions can use a variety of built-in operators (see below) and may 
contain function calls. The order in which arguments to functions and 
operands to most operators are evaluated is unspecified. The evaluations 
may even be interleaved.

PS: defines sind nicht schön. :-)

[Karl H. (kbuchegg)]

Jürgen G. schrieb:
>>> Vor dem Aufruf einer Funktion werden die Argumente von links nach rechts
>>> berechnet.
>>wer sagt das?
>
> In C ist (fast) alles definiert bzgl. Auswertung von Ausdrücken.

das genaue Gegenteil ist der Fall.
In C ist es fast immer so, dass sich der Compiler die 
Auswertereihenfolge aussuchen kann.
Verwechsle nicht 'Operator precedence' mit Auswertereihenfolge. Das sind 
2 verschiedene Dinge.

In
    a + b * c

ist zwar definiert, dass b mit c multipliziert werden muss und zu diesem 
Zwischenergebnis a dazuaddiert wird. Das sagt aber nichts darüber aus, 
in welcher Reihenfolge die Werte von a, b und c berechnet werden.

    c = foo() + bar()

dem Compiler steht es frei, die Funktionen foo bzw. bar in der 
Reihenfolge aufzurufen, die ihm am besten in den Kram passt.

[Jürgen W. (lovos)]

> Verwechsle nicht 'Operator precedence' mit Auswertereihenfolge. Das sind
> 2 verschiedene Dinge.

Ich habe mich immer darauf verlassen, dass Ausdrücke wie

*p++ != 0 && *q++ != 0

richtig von links nach rechts berechnet wurden.

Bei bestimmten Ausdrücken (wie oben) ist das auch garantiert, siehe

http://en.wikipedia.org/wiki/Sequence_point
 Between evaluation of the left and right operands of the && (logical 
AND), || (logical OR), and comma operators. For example, in the 
expression *p++ != 0 && *q++ != 0, all side effects of the 
sub-expression *p++ != 0 are completed before any attempt to access q.

[Peter II (Gast)]

hier ein schönes beispiel:

#include <stdio.h>

void test( int a, int b, int c )
{
   printf("%d, %d, %d \n", a, b, c );
}

int main()
{
   int i = 0;
   test( i++, i++, i++ );
};

[Karl H. (kbuchegg)]

Jürgen G. schrieb:
>> Verwechsle nicht 'Operator precedence' mit Auswertereihenfolge. Das sind
>> 2 verschiedene Dinge.
>
> Ich habe mich immer darauf verlassen, dass Ausdrücke wie
>

> *p++ != 0 && *q++ != 0
>

> richtig von links nach rechts berechnet wurden.
>
> Bei bestimmten Ausdrücken (wie oben) ist das auch garantiert, siehe
>
> http://en.wikipedia.org/wiki/Sequence_point
>  Between evaluation of the left and right operands of the && (logical
> AND), || (logical OR), and comma operators. For example, in the
> expression *p++ != 0 && *q++ != 0, all side effects of the
> sub-expression *p++ != 0 are completed before any attempt to access q.

Richtig.

Aber in

  ... strncmp( line, s1="Number=", strlen(s1) ) ....

gibt es nur 1 Sequence Point. Und der sitzt unmittelbar vor Aufruf der 
Funktion, nachdem die Funktionsargumente ausgewertet sind. Wie und in 
welcher Reihenfolge die Argumente ausgewertet werden, ist nun mal nicht 
definiert. Der Sequence Point legt nur einen Zeitpunkt fest, zu dem alle 
Aktionen abgeschlossen sein müssen, er sagt aber nichts über 
Reihenfolgen der Aktionen zwischen den Sequence Punkten aus.

Oh. Zusätzlich noch.
Dieses Komma hier, welches Argumente voneinander trennt, ist NICHT der 
Komma-Operator. D.h. der Passus hier ...
> Between evaluation of the left and right operands of the && (logical
> AND), || (logical OR), and comma operators.
                             ****************
... ist nicht anwendbar.

[Klaus W. (mfgkw)]

Jürgen G. schrieb:
> strncmp(line,s1="Number=",strlen(s1))==0)

du bist dir im Klaren darüber, daß da höchstens ein Zeichen verglichen 
wird (vom Fehler mit der undfinierten Reihenfolge abgesehen)?

Und dann auch noch gerade 1, wenn s1 eh leer ist, es also nichts zu 
vergleichen gibt. Wenn s1 mehr als Zeichen Länge hat, wird nichts 
verglichen.
Wer programmiert sowas eigentlich?

[Stefan E. (sternst)]

Klaus Wachtler schrieb:
> Und dann auch noch gerade 1, wenn s1 eh leer ist, es also nichts zu
> vergleichen gibt. Wenn s1 mehr als Zeichen Länge hat, wird nichts
> verglichen.

Da hat wohl jemand die Klammer hinter strlen(s1) übersehen. ;-)

[Peter II (Gast)]

Klaus Wachtler schrieb:
> du bist dir im Klaren darüber, daß da höchstens ein Zeichen verglichen
> wird (vom Fehler mit der undfinierten Reihenfolge abgesehen)?

kannst du mal bitte sagen warum? Wenn in s1 zufällig ein string mit der 
richtige länge drin steht sollte es doch passen oder nicht?

[Klaus W. (mfgkw)]

Stefan Ernst schrieb:
> Da hat wohl jemand die Klammer hinter strlen(s1) übersehen. ;-)

ok, eingesehen :-(

(wobei ich dann aber noch anmerken möchte, daß es nicht besonders lesbar 
ist, alles ohne Leerzeichen aneinander zu kleben...)

[Mark B. (markbrandis)]

Jürgen G. schrieb:

>   char line[0x100];
>   char *s1=NULL;
>   int noMan;
>   ...
>   if (strncmp(line,s1="Number=",strlen(s1))==0) {
>     noMan = atoi(line+strlen(s1));
>   }
>   else if (strncmp(line,s1="Value=",strlen(s1))==0) {...}
>   else if (strncmp(line,s1="Pos=",strlen(s1))==0) {...}

Wer so programmiert, gehört doch verprügelt. Warum diese Krankheit, 
möglichst viel in eine Zeile reinquetschen zu wollen? Es ist ein 
Märchen, dass der Code dadurch effizienter wäre.

Und wer sowas schreibt, der benutzt auch sprintf() statt snprintf() und 
macht mit Strings alle möglichen sonstigen Fehler, die potenziell zu 
Buffer Overflows führen.

[Karl H. (kbuchegg)]

Jürgen G. schrieb:

Das wollte ich in der Früh schon schreiben:

> PS: defines sind nicht schön. :-)

sie sind aber an dieser Stelle das Mittel der Wahl. Compile-Time 
Konstanten direkt im Code zu haben, ist noch schlimmer.
Ok. Man könnte höchstens noch über ein

 const char * const NumberTag = "Number=";

nachdenken, damit man dann auch noch was fürs Auge bzw. für den Debugger 
bietet.

[Sebastian L. (Gast)]

Hi,

schließe mich auch an das defines nicht schön sind aber dazu gibt es 
doch auch alternativen

Wie wäre es dann mit Konstanten

   char *const num="Number=";
   char *const val="Value=";
   char *const pos="Pos=";

   char line[0x100];
   int noMan;
   ...
   if (strncmp(line,num,strlen(num))==0) {
     noMan = atoi(line+strlen(num));
   }
   else if (strncmp(line,val,strlen(val))==0) {...}
   else if (strncmp(line,pos,strlen(pos))==0

[Vlad T. (vlad_tepesch)]

Sebastian L. schrieb:
Ich würde lieber folgendes schreiben:

   char const num[]="Number=";
   char const val[]="Value=";
   char const pos[]="Pos=";

weiß aber nicht ob das selbe rauskommt, kann sein, dass der Compiler das 
sowieso optimiert, glaub ich aber eher nicht.

optimiert der compiler strlen() für Konstanten?
so sollte es auf jeden Fall effizient sein:

/// string muss ein array (kein Pointer!) sein
#define STRLEN(string) (sizeof(string)-1)

...

   char line[0x100];
   int noMan;
   ...
   if (strncmp(line,num,STRLEN(num))==0) {
     noMan = atoi(line+STRLEN(num));
   }
   else if (strncmp(line,val,STRLEN(val))==0) {...}
   else if (strncmp(line,pos,STRLEN(pos))==0

[Vlad T. (vlad_tepesch)]

Btw:

Ich glaube nicht, dass der Code das macht, was der Autor im Sinn hat:

Jürgen G. schrieb:
> if (strncmp(line,s1="Number=",strlen(s1))==0) {
>     noMan = atoi(line+strlen(s1));
>   }

wahrscheinlich meiner er eher:

  char const num[]="Number=";

  if (strstr(line,num) == line) {
    noMan = atoi(line+STRLEN(num));
  }

gibts auch eine "sichere" Variante von strstr?


Edit:
Erklärung:
strncmp gibt nur 0 zurück, wenn beide Strings komplett gleich sind, also 
nicht, wenn nur der Anfang überein stimmt.


Edit Edit:

Ok - vergesst es, es werden ja nur die ersten Zeichen verglichen - 
sollte also doch passen.

[Jürgen W. (lovos)]

char *const num="Number=";

bedeutet: "num" ist ein konstanter Zeiger auf ein Char-Array.
d.h. der Pointer darf nicht verändert werden, aber der Speicher, wo er 
hinzeigt schon.

char const * num="Number=";

bedeutet: "num" ist ein Zeiger auf ein konstantes Char-Array.
d.h. der Pointer-Wert darf verändert werden, aber der Speicher, wo er 
hinzeigt, darf nicht verändert werden.

Man könnte auch noch schreiben:

const char * const num="Number=";

das wäre dann wirklich "konstant"

[Jürgen W. (lovos)]

Hier ein kleines Test-Programm zum obigen Beitrag (Unterschied zwischen 
"const *" und "* const").

#include <stdio.h>
#include <stdlib.h>

int main() {
const char *a = malloc(10);
char * const b = malloc(10);
const char * const c = malloc(10);

a=NULL;
b=NULL; //error: assignment of read-only variable ‘b’
c=NULL; //error: assignment of read-only variable ‘c’

a[0]=0; //error: assignment of read-only location ‘*a’
b[0]=0;
c[0]=0; //error: assignment of read-only location ‘*c’
}

[Vlad T. (vlad_tepesch)]

Jürgen G. schrieb:
> Man könnte auch noch schreiben:const char * const num="Number=";
> das wäre dann wirklich "konstant"

man könnte sich auch den Speicher für den Zeiger sparen und es so 
machen, wie ich oben geschrieben habe.
Beitrag "Re: Compiler warning: operation on ‘x’ may be undefined"