7

Recently I found a great example of why C-style casts are bad. We start with a following class implementing multiple COM interfaces (I have two for brevity, but there can be ten in real life):

class CMyClassInitial : public IInterface1, public IInterface2 {
    //declarations omitted
};

HRESULT CMyClassInitial::QueryInterface(REFIID iid, void** ppv)
{
    if( ppv == 0 ) {
       return E_POINTER;
    }
    *ppv = 0;
    if( iid == __uuidof(IUnknown) || iid == __uuidof(IInterface1) ) {
       *ppv = (IInterface1*)this;
    } else if( iid == __uuidof(IInterface2) ) {
       *ppv = (IInterface2*)this;
    } else {
       return E_NOINTERFACE;
    }
    AddRef();
    return S_OK;
}

The above implementation uses C-casts for adjusting pointers to account for multiple inheritance. They even work as static_casts - this pointer value will be properly adjusted.

Now we copy-paste (or should I say reuse code of?) the same QueryInterface() implementation to some other very similar class.

class CMyClassModified : public IInterface1 {
    //declarations omitted
};

and leave the implementation the same. The new class doesn't inherit from IInterface2 anymore but

} else if( iid == __uuidof(IInterface2) ) {
*ppv = (IInterface2*)this;
}

will compile just fine and C-style cast will act as reinterpret_cast - this pointer value will be copied unchanged. The caller will obtain a pointer to an object that doesn't actually implement IInterface2 - straight way to undefined behavior. Such problems can be hard to spot in a huge database and when there're many (not two as in my example) interfaces.

If static_cast was used that would not have happened - the compiler would emit an error trying to compile

*ppv = static_cast<IInterface2*>(this);

IMO that's a harsh enough example of how using C-style casts can cause serious problems.

What other examples are there?

Community
  • 1
  • 1
sharptooth
  • 167,383
  • 100
  • 513
  • 979
  • A great gotcha, but I'm not entirely sure this is suitable to SO. It seems very discussiony. At best it's a community wiki. – tenpn Feb 03 '11 at 10:25
  • @tenpn: I don't see what can be discussed here - just an example of shooting oneself in the leg in C++. – sharptooth Feb 03 '11 at 10:28
  • 1
    @sharptooth but that's not a question, is it? – tenpn Feb 03 '11 at 10:48
  • @tenpn: Hmm... I ask what other examples are there. Is that not a question? – sharptooth Feb 03 '11 at 10:59
  • 2
    That can also be taken as one example of why copy-pasting code (especially without thoroughly reading through it) is generally a bad idea. – Rune Aamodt Feb 03 '11 at 11:06
  • @codebolt: Well, yes, but it's not that easy to spot the problem if you have 7 interfaces and 2K lines of code by reading the code. Just using `static_cast` would have caught the problem immediately. – sharptooth Feb 03 '11 at 11:12
  • @sharptooth: Can your rephrase this so that your example can be moved down to an answer? – Bill the Lizard Feb 03 '11 at 12:18
  • @Bill the Lizard : I have no idea of how that can be done without violating the "show us what you have done before asking a question" rule. – sharptooth Feb 04 '11 at 06:52
  • The problem with this example is that bonehead copy-paste can *always* cause rebind problems. This doesn't form any part of an argument that C-style casts are bad -- or, rather, this is just as strong an argument for requiring everybody prefix every variable name with the source-file name, line number and scope identifier of its definition. – jthill Jan 13 '13 at 07:27

2 Answers2

2

This FAQ item sums up why C-casts are bad.

Any C-style cast is potentiality a bomb, since they are hiding conversion warnings and errors by silencing the compiler.

Since you wanted an example, here it is:

int main()
{
    float a = 0.123;
    double *b = ( double* ) &a;
    *b = 0.123;
}
Olivia Stork
  • 4,660
  • 5
  • 27
  • 40
BЈовић
  • 62,405
  • 41
  • 173
  • 273
1

A very simple example:

class ClassB;//only forward declaration, no real declaration included

Class A * a;
Class B * b;
a = (ClassA *)b;

The cast will always be silently successful if there's only forward declaration of ClassB. It doesn't care if ClassB is derived from ClassA. And it will also be wrong when ClassB is not only derived from ClassA:

class ClassB:public SomeOtherClass, public ClassA {};
TerryYin
  • 174
  • 2
  • 6