Tales from the debugging land

February 21, 2010

Here’s a funny bug I wanted to share with you (where “you” means all audience with bare minimum C++ understanding). I’ve been working today to add a putter interface to Kolf 2. (Attention: There are three Kolfs at the moment. I did previously explain why.) After fixing one or two crashes and fighting against Qt’s partly abstruse event forwarding (why do I have to setAcceptHoverEvents() in order to get mouseMoveEvent()s?!?), everything worked as it should, though there was one problem left: A mouse move event needed around 1 second to be processed. Callgrind to the rescue!

clear-priorities

Okay, I admit that I had not expected the result to be so very clear. Let’s have a look at that method then:


void Kolf::SimplePutter::updateAppearance()
{
  if (!m_activeView)
    m_lineItem->hide();
  else
  {
    m_lineItem->show();
    //calculate line
    QPointF lineEnd = m_cursorPosition;
    for (int i = 0; i < m_direction; ++i)
      rotateBy90Degrees(lineEnd);
    //update line
    m_lineItem->setLine(QLineF(QPointF(), lineEnd));
  }
}

Looks quite compact, doesn’t it? Hm, rotateBy90Degrees is called multiple times, perhaps that’s the bottleneck:

void rotateBy90Degrees(QPointF& p)
{
  qreal x = p.x();
  p.rx() = p.y();
  p.ry() = -x;
}

Doesn’t look like it could be any more efficient. But even if it was unefficient, m_direction always has a value smaller than four, because everything else is nonsense. Anyway, let’s have a look at what value is in m_direction:

qDebug() << m_direction; //just before the for loop
//output:
135552768
135552768
135552768
135552768
135552768
135552768
135552768

A quick look in the constructor… Bah, I hate you, C++! Not only do you initialize int member variables with random values, but you also initialize them with random values that are always dividable by four so I don’t notice!

Advertisements

11 Responses to “Tales from the debugging land”

  1. Stefan Majewsky Says:

    Please note that I am well aware of the fact that C++ does not really initialize member variables, but rather just leaves the value that is already in the memory at that position. I also know that these values are dividable by four because these probably were pointer adresses. I did not write this into the article itself because I did not consider this funny. 😉

  2. Ben Boeckel Says:

    -Weffc++ 🙂 -pedantic is also helpful. I’m currently working on a project where we have pretty much all the warnings enabled along with -Werror. This has resulted in much cleaner code and fewer stupid mistakes since we must deal with initializing every member variable in classes.

    • Stefan Majewsky Says:

      Nice tip, though I do not know yet how I can enable that in CMake. Any clues, anyone?

      • Dennis Nienhüser Says:

        In marble:
        $ grep -B 1 -i pedantic CMakeCache.txt
        //Determines if we should compile with -Wall -Werror.
        PEDANTIC:BOOL=FALSE

        Haven’t checked other projects though. There’s also CMAKE_CXX_FLAGS_DEBUG which can be used for that (variable name must match your build type).

        • Stefan Majewsky Says:

          Hm, it’s always time to learn something. Thanks for the hint, although I doubt I’ll leave -Weffc++ enabled. It gives me bazillions of warnings that some classes have pointer data members, but do not override copy-constructor or operator=. Nearly all these classes derive from QObject in some way, so this is a non-issue.

  3. Andreas Says:

    If you had not found it with callgrind you’d have found it with memcheck 🙂

  4. Christoph Bartoschek Says:

    You should be happy that the values are not initialized. It sometimes helps to find errors in the program logic.


  5. I wish gcc would complain (warn) about unitialized members at the end of the constructor.

  6. Frank Says:

    To get mouse move events when no button is pressed, you just have to call setMouseTracking( true );

  7. Girish Says:

    Frank, I think his code refers to a QGraphicsItem (and not QWidget).

    “why do I have to setAcceptHoverEvents() in order to get mouseMoveEvent()s?!” – I don’t think mouse move is affected by the flag at all. It only affects delivery of hoverEnter/Leave/MoveEvent. mouseMoveEvent is only meant to handle the case when atleast one button is pressed.


Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s