Simple bugs that crashed my mind

This post is more about a memorial of my errors or those I found during the years than a tutorial or research entry. Even though, I’ll try to give an explanation about why it happened and what we try to take into account for the future. They’re usually simple errors but hard to find when you don’t know what you’re looking for. Some times it crashes but other times it just causes unexpected behavior.

I’ve some for C++ and other about Qt. I think it’s quite interesting to see both of them because their types and reasons are quite different. While in C++ we have full control about memory allocation and deallocation in Qt we have the re-parenting issue.

Simple bugs in C++

Old C-style arrays are deprecated

The other day I was having some random crash with what look like a corrupted stack, but everything was fine. I had fill an array of doubles in a loop and it was coded in old C-style:

Despite the fact of having the number 12 hard coded, don’t you see anything weird? Yes, the parentheses! Array is in fact a pointer to a double with value 12 and then we were iterating over the unknown. Why we don’t use the std::array type? It’d be easy, nice and clean:

As you can see: easy, clean and without any problem!

When you mix up pointer to an object with references of an object

This has happened several times because it’s not intuitive what we want to achieve. The code compiles and runs properly, almost. I’ve a structure as a singleton where I take one object, update or modify it and then I commit the changes to the database. I’ll show just the part of the code necessary:

First of all, we’ll get close to the functionality of the commit method. There we get a clip, modify it and the commit. The clip is modified, and commit properly to the database. The problem is that I was doing a copy of that clip, I was not modifying the clip IN the DataStructure. The solution was easy:

The problem now is: what happens if id is not found? We return a reference of a temporary object, so when we go out of the post method scope it is dereferenced and removed. So our pointer is not valid anymore. Obviuosly if we want the modification and commit behavior we have two options: do it inside the DataStructure or return a pointer or nullptr instead:


A variant of the previous one

In the code above I said I have a method postPtr (int id) that returns a pointer to a Post of the inner map. The reason I have it is to allow other programmers to access that structure and modify the data. If a user wants a specific Post but doesn’t want to modify it, I have another method called getPost (int id) const that returns a const reference to a copy of the object with that id and if it’s not found it returns a new object so the user can check if is valid.

I say so because today I found a crash in the application because it was supposed to user getPost () instead of the pointer return. Every call inside that particular method was only to get information from the Post instead of modifying the data. In that method it should be used the constant method. But instead, it was used the pointer without checking if it was nullptr.

In addition, the method had two calls to a get method that in addition to return the value it applies an internal operation that modifies it. So:

  1. The reason was crashing is because the method called changed from getPost to post. From const reference of an object to a pointer to an object (or nullptr!)
  2. The reason of that change is because we need to make some modifications in that Post object in that method but the change log only shown that get methods where called.
  3. One get method wasn’t a const method as its name indicate. It was doing more than the name of the method

So remember: the name of the method must explain what the method does and the method only must do one thing at a time. If it’s a get method it must be const always.

Simple bugs in Qt

Easy to create, easy to copy

One of the main problems of Qt is that we are use to really on copy-on-write (COW) optimization and therefore we assume that we can write atrocities like the following:

I really don’t know from where I start, I just so wrong in all ways that the best way is just to remove everything and start from scratch. The variable <em>mSet</em> is a field of the class, as you can see by the allocation is a pointer to a QSet of integers.So, let’s start with the first line or better say for the absence of a previous one:

We just had the first line and we cause the first memory leak. After that we call the static method fromVector with a temporary object that is destroyed after using it. The same happens with the temporary object from that method when we pass the parameter to the fromList. So, in a single line we have 3 expensive object creations (temporary QVector, temporary QList and temporary QSet). In this particular case we cannot use std::move since the parameter the static methods are waiting for is a const reference. In the following line we take that temporary object and pass it as a constructor parameter to create a new QSet and it performs a deep-copy. Wonderful!

Please, take into account that most of the times is easy to add a single loop than several calls to static methods chained:

The loop of the death

This particular bug has happened to me several times and the reason is simple: too fast writing code. Remember to double check your call al the time! In the following code I had to process an event, do some operations and then pass to the base class so it can continue its flow:

It was solved before was committed but is so embarrassing I wanted to have it here.

Signals with params between threads

In Qt as well in other frameworks or libraries we have signals that allows us to send parameters using some sort of callback system (we’ll see the inner implementation in further post). It means that we can also use it to send information between threads. But we always must remember what are we sending. In the following code I’ve avoided all the noise of the macros and declarations so we can focus on the signal’s parameter:

This is a potential failure. If we are sending constant references to an object between threads is possible that when the signal is processed, the data is no longer valid. We always must send copies of an object or if we want to share data, a shared pointer:

And that’s all… for the moment!

Leave a Reply