Creating and checking a pointer to an "if" expression

Question

Is this code correct?

void foo ( int* p ) { if ( int* p2 = p ) // single "=" { *p2++; } } 

I always didn’t think about it, but recently I saw such a code from a colleague of my sources.

What if p is NULL? MS VS 2008 works correctly, but shows "warning C4706: assignment in conditional expression."

Thanks.

Answer 1

Raising warning C4706 , the compiler simply interrogates whether you really intended to write if ( int* p2 == p ) instead of if ( int* p2 = p ) , as shown.

According to 6.4 of the 2003 C ++ standard, if ( int* p2 = p ) is legal:

The value of the condition [ if (condition) ], which is initialized in a statement other than the switch , is the value of the declared variable is implicitly converted to type bool .

If p is NULL , the condition ( int* p2 = p ) fails because the value of p2 is 0 and therefore implicitly false , and *p2 does not increase.

Answer 2

A warning assignment within conditional expression usually issued by compilers to prevent you from writing

  if (a = b) { 

where did you mean

  if (a == b) // big difference! { 

In your example, the warning “assignment” is actually fictitious , since it is not actually an assignment, but rather an initialization:

  { int *p2 = p; if (p2) { } } 

and there is no risk that you really would like a syntax error ( int *p2 == p ?!) instead :)

The rest of your post is perfectly correct with C ++ 03 (and later) and just does what ¹ says.

---

¹ (which in terms of long-term effect is not so much because

• p2 is dereferenced without any action with it
• p2 increases without any action with it,

but i think only sample code? In case this is not obvious, *p2++ equivalent to *p2; p2++; *p2; p2++; )

Answer 3

Formally correct (in C ++, but not in C). Resolution definition as a condition added with dynamic_cast to support things like:

 if ( Derived* p = dynamic_cast<Derived*>( ptrToBase ) ) { // ... } 

The argument was that there would never have been a moment when p was in scope but not null. After if , p no longer in scope.

The argument does not contain water, because, on the one hand, if you add else , in the else block p is in scope and null. And the justification for limiting the volume is really not very strong anyway, because if the function is long enough for it to matter, the function is too long and too complicated. This design supports things like:

 if ( Derived1* p = dynamic_cast<Derived1*>( ptrToBase ) ) { // ... } if ( Derived2* p = dynamic_cast<Derived2*>( ptrToBase ) ) { // ... } // ... 

But this is really not what you want to do in good code.

In general, I would avoid the construct by replacing it with something like:

 int* p2 = p; if ( p2 != NULL ) { // ... } 

It is more explicit and readable. This means that p2 is in scope outside of if , but I would say that if this causes any problems, the function itself needs to be refactored because it is too long and too complex.

EDIT:

As for the warning: this is complete idiocy by the compiler. There are no assignments in the conditional expression: there is no assignment, period, and there is no conditional expression "the condition in the original is an announcement, not an expression. If the compiler wanted to warn something (in style because he agrees with me that this is a bad style), then this should warn of implicit conversion to bool . (And, of course, whether this warning should be emitted should be controlled by the option. The compiler should compile the language, and not impose a style, if only one asks it to explicitly warn about special style problems.)

Answer 4

the code is error prone (why not split it into 2 statements), but otherwise it is correct. if you divide this into:

 void foo ( int* p ) { int* p2 = p; if ( p2 ) { *p2++; } } 

it is more readable and will not give you a warning. You can see that this is also correct (in the case of null, you just assign p2 with a null value, which is good).

Answer 5

If p is NULL, the condition is not satisfied. So the code is fine.

Answer 6

This code seems weird. This is correct, but I do not see in what cases it is useful.
These are less readable and then simple alternatives (for example, the mentioned Oren S.), and without the need defines a new variable to increase (* p). The p parameter is already a new variable on the stack, why create another?

Answer 7

I think grammatically the code has no problems. But this is not a very good style. If you definitely know that there is a single "=", you can also read like this:

 if ((int* p2 = p) != NULL) { *p2++; } 

I prefer the pointer to be compared to NULL.

Answer 8

Take a look at int * p, as this "p is a pointer to int". And "p2 is also a pointer to int". You make p2 a pointer to the pointer pointed to by p when you say p2 = p. Statement * p2 ++; increment the address by 4 and point to something other than the one you originally pointed to. Whati should be a 32 bit integer. The pointer is incremented by 4 on the 32-bit compiler. If you use (* p2) ++ wrapped in a parenthesis, it will increment the integer that the pointer points to.

Yes, it works ... wierd.

  int ClassPtr :: MethodA (int * p)
 {
     if (int * p2 = p)
     {
        (* p2) ++;
        return * p2;
     }
           return 0; 
 }