Strange template behaviour with strings

[meLlamanJefe]

I have written a Queue class for our use that stores pointers to what I call RequestObjects. RequestObjects are a base class from which other types inherit and this allows the queue (or deque in this case) to hold values of different types. When popping values off the deque, the values are then typecasted back to their useful derived type. So far the class has worked well except with strings. When string values are typecasted back to be read off the deque the program segfaults. Here's a quick example of the code and the result of the unit test at the bottom:

// Queue.h
   class RequestObject
   {
     public:
      // Constructor
      RequestObject(){ mType = -1; }
      virtual ~RequestObject(){}

      int GetType() const { return mType; }
      void SetType(int i) { mType = i; }

     private:
      int mType;
   };

   template <class T>
   class SingleValueReq : public RequestObject
   {
      public:
         SingleValueReq(){ mValue = NULL; }
         SingleValueReq(T b){ mValue = new T(b); }
         virtual ~SingleValueReq(){ delete mValue; }

         void SetValue(T value){ delete mValue; mValue = new T(value); }
         T GetValue(){ return(*mValue); }
      private:
         T    *mValue;
   };

   class QueueObject
   {
   public:
       QueueObject(){}
       ~QueueObject(){}
  
       // Public Methods
       bool Empty();
       RequestObject *Pop();
       void Clear();
 
      // Add mechanism
       template <typename T>
       void Add(int type, T value)
       {
          SingleValueReq<T> *O = new SingleValueReq<T>(value);
          O->SetType(type);
          Q.push_back(O);
          // debug code
          O = (SingleValueReq<T> *)Q.back();
          std::cout << "Add " << O->GetValue() << std::endl;
       }

   private:
       std::deque<RequestObject *> Q;
   };

// From file Queue.cpp
RequestObject *QueueObject::Pop()
{
    RequestObject *O = Q.front();
    Q.pop_front();
    return(O);
}

bool QueueObject::Empty()
{
   return(Q.empty());
}

// From test program QueueTest.cpp
int main(int argc, char *argv[])
{
   QueueObject mQ;

   mQ.Add(10, -1.0);
   mQ.Add(11, true);
   mQ.Add(12, -1);
   mQ.Add(13, false);
   mQ.Add(14, "StringTest!!!");

   while(!mQ.Empty())
   {
      RequestObject *O = mQ.Pop();

         switch(O->GetType())
         {
            case(10):
               {
                  SingleValueReq<double> *ip = (SingleValueReq<double> *)(O);
                  cout << "Value = " << ip->GetValue();
               }
               break;
            case(11):
               {
                  SingleValueReq<bool> *ip = static_cast<SingleValueReq<bool> *> (O);
                  cout << "Value = " << ip->GetValue();
               }
               break;
            case(12):
               {
                  SingleValueReq<int> *ip = static_cast<SingleValueReq<int> *> (O);
                  cout << "Value = " << ip->GetValue();
               }
               break;
            case(13):
               {
                  SingleValueReq<bool> *ip = static_cast<SingleValueReq<bool> *> (O);
                  cout << "Value = " << ip->GetValue();
               }
               break;
            case(14):
               {
                  SingleValueReq<string> *ip = (SingleValueReq<string> *) (O);
                  cout << "Value = " << ip->GetValue();
               }
               break;
            default:
               break;
         }
      cout << endl;
   }

   return(0);
}

// Sample output from QueueTest.cpp
]./QueueTest
Add -1
Add 1
Add -1
Add 0
Add StringTest!!!
Value = -1
Value = 1
Value = -1
Value = 0
Segmentation fault

I've debugged this as far as my understanding can take me but cannot figure out why the string becomes corrupted but the other values don't. Any help or ideas?

Thanks!

-Marco

[weaknessforcats]

Typecasting in C++ means a) you are calling a relic C function with some soirt of void* argument or b) your C++ design is faulty.

In this case, the design is faulty.

I suggest you use the Visitor design pattern.

[meLlamanJefe]

I appreciate you pointing out that the design is faulty and I will take a look at the visitor design pattern as you suggest. However, being new at this your answer does not really mention which part is faulty and why. Could you explain that?

Thanks.

[weaknessforcats]

It's because of the typecast.

Whenever you have to tell a C++ compiler what the type is, there is something fishy. Plus ion order to write the typecast in the firslt place you need to know the type you are casting to. That means hard-coded types in your programs and that means the program may need to be altered as new derived classes are added. If there are a lot of copies of this code in use, then a lot of changes will be required to add you new derved type.

However, I compiled your code and ran it with no seg fault using Visual Studio.NET 2008.

[meLlamanJefe]

Got it. I completely understand the limitation with respect to the hard coding of the code. I hop to understand the visitor pattern and implement it if that can make our code more flexible. I'm glad but confused to hear that the code worked for you. While it is not the cleanest code I did not see anything in it that told me it would not work which is why we were using this design. We are running in Linux and are building our code using gcc 4.1.2 so I'm guessing it is a problem with the compiler. Oh well...

Thanks for your help!

[meLlamanJefe]

Just a quick update as reference for others that may look at this post. I finally understood the reason why the code above segfaults and it was not due to a bug in gcc.

The problem is in line 79. A quoted string passed to a method in C++ is not passed as a std::string but as a const char *. In newer languages like C# or Java I think the quoted text in line 79 would have been treated as a string object. So the first problem was in line 113 where the object returning from the Queue was templated into a string while in fact being a const char array. Finally in line 114 when the GetValue method tried to access the value in the SingleValueReq object the program crashed. The other problem is that passing a const char array is extremely unsafe because there is no information on the length of the array and thus the GetValue method could easily overrun the array's limits and return garbage.

The easy fix was to create a string object with the char array value and then add it to the Queue. The rest of the code then works properly.

The CORRECT fix is to read up on the visitor pattern and re-implement the request objects in that way. (I'm still working on that but hope to get it working soon).

thanks!