QSharedDataPointer for private data: how to write a good copy operator where a copy may be deleted without getting a segfault?

Lenz Moser

I needed to implement a copy operator for a class that contains a QSharedDataPointer to the private data. This works, but only as long as I do not delete one of the copies. Contrary to expectation, the reference counter does not seem to be incremented by copying the pointer.
Instead, the pointer is the same in both instances. If one instance is deleted, using the other copy results in a segmentation fault.
It seems that by implementing an assignment operator, the move operator (&&) is used in QSharedDataPointer. How can I fix this? In the copy constructor I do the same assignment (d = d.other) and here it does what's expected.

Below is an example of this mechanism.

main.cpp:

#include "Nix.h"
int main( int , char **  )
{
  Nix *nix1 = new Nix(1, "Hello, I'm one");

  Nix *nix2 = new Nix();
  nix2 = nix1;
  nix2->add(2, "Hello, I'm two");

  Nix *nix3 = new Nix(*nix2);
  nix3->add(3, "Hello, I'm three");

  qWarning() << "nix1: " << nix2->display(1) << nix1->display(2) << nix1->display(3);
  qWarning() << "nix2: " << nix2->display(1) << nix2->display(2) << nix1->display(3);

  delete nix2; // if nix1 is deleted, accessing nix2 will produce a segmentation fault

  qWarning() << "nix3: " << nix3->display(1) << nix3->display(2) << nix3->display(3);

  // here comes the segmentation fault:
  qWarning() << "nix1: " << nix1->display(1) << nix1->display(2) << nix1->display(3);
}

Nix.h

#include <QString>
#include <QSharedDataPointer>

class NixPrivate;

class Nix
{
public:
   Nix();
   Nix(int a, QString b);
   ~Nix();
   Nix(Nix const &other);
   Nix &operator=(const Nix &other);

  QString display(int x);
  void add(int a, QString b);

private:
  QSharedDataPointer<NixPrivate> d;
};

Nix.cpp

#include "Nix.h"

#include <QMap>
#include <QString>
#include <QSharedData>

class NixPrivate : public QSharedData
{
 public:
   QMap<int, QString> myMap;
};

Nix::Nix()
  : d(new NixPrivate())
{}

Nix::Nix(int a, QString b)
  : Nix()
{
  d->myMap[a] = b;
}

Nix::~Nix()
{}

Nix::Nix(Nix const &other)
  : Nix()
{
  d = other.d;
}

Nix &Nix::operator=(const Nix &other)
{
  d = other.d;
  return *this;
}

QString Nix::display(int x)
{
  if (d->myMap.contains(x))
  {
    return d->myMap.value(x);
  }
  else
  {
    return "not found";
  }
}

void Nix::add(int a, QString b)
{
  d->myMap[a] = b;
}

SGaist

Hi,

nix2 = nix1;

Here you are just replacing a pointer with another pointer. There's not reference counting involved.
Reference counting works on objects not on pointer to objects.

Lenz Moser

Oh yes, thank you. That was a total thinking failure. What I actually wanted to test was something like that:

{
     Nix nix4(4, "Hello, I'm four");

     *nix1 = nix4;
}
qWarning() << "nix1: " << nix1->display(4);

And that works.

*nix2 = *nix1; delete nix1

works too

Thanks, solved!