bug1529.cpp

1    #include <stdio.h>
2    #include <string.h>
3
4    struct X {
5        char *p;
6        X() { p = new char[100]; strcpy( p, "hello world" ); }
7        X( const X & x )
8            { p = new char[ strlen(x.p) + 1 ]; strcpy( p, x.p ); }
9        X &operator=( const X & x )
10           { delete[] p;  p = new char[ strlen(x.p) + 1 ];
11             strcpy( p, x.p ); return *this; }
12       ~X() { delete[] p; }
13       };
14
15   void g( X &x1, X &x2 ) { x1 = x2; }
16
17   int main()
18       {
19       X a;
20       g( a, a );
21       printf( "%s\n", a.p );
22       return 0;
23       }

In testing his new class X, the programmer was suprised not to see the familiar 'hello world' message. What did he do wrong?

bug1529.cpp lint Output

--- Module:   bug1529.cpp
                                         _
          strcpy( p, x.p ); return *this; }
bug1529.cpp(11) : Warning 1529: Symbol 'X::operator=(const X &)' not first
    checking for assignment to this

Reference Manual Explanation

 
1529   'Symbol' not first checking for assignment to this  -- The
       assignment operator does not appear to be checking for assignment
       of the value of a variable to itself (assignment to this).
       Specifically FlexeLint/PC-lint is looking for one of:

                 if( &arg == this )
                 if( &arg != this )
                 if( this == &arg )
                 if( this != &arg )

       as the first statement of the function.

       It is important to check for a self assignment so as to know
       whether the old value should be subject to a delete operation.
       This is often overlooked by a class designer since it is
       counter-intuitive to assign to oneself.  But through the magic of
       aliasing (pointers, references, function arguments) it is
       possible for an unsuspecting programmer to stumble into a
       disguised self-assignment [12, Item 17].

       If you are currently using the following test

                 if( arg == *this)

       we recommend you replace this with the more efficient:

                 if( &arg == this || arg == *this)