[Development] QLogger questions and proposal

[Mike Zraly]

>
> Hi guys,
> QLogger is now available as a module:
> git clone http://codereview.qt-project.org/p/playground/qlogger
> At this moment %{category} syntax in MessagePattern can't be used because
> we need to wait for a qtbase update.
> Therefore we created a QLogger module that works with the current qtbase.
> The category will be attached in the log message but only for qCDebug,
> qCWarning and qCCritical.
> This will be changed as soon qtbase is updated.
> Switching on or of  qDebug, qWarning and qCritical works.
> Checkout the example in QLogger module.
> Cheers,
> WB


Very nice.  I've seen so many people try to create a configurable tracing
facility for Qt, it's a relief to see something being standardized.

That said, I do have some questions, and a proposal:

The questions:

First, what happens if QLogger is used inside a shared library, that
library gets unloaded, and then settings are updated?

>From reading the code, it looks like once qCDebug is called inside the
shared library the address of the associated QLoggingCategoryObject will be
added to the global QLoggingPrivate's _registeredCategories data member.
 When the library is unloaded that address will no longer be valid.  If the
settings are then updated that invalid address will be passed to
QLoggingPrivate::updateCategory.  That method will de-reference the invalid
address and generate a segmentation fault.

Second, how can I use qCDebug with a QLoggingCategory object defined inside
a class or function which takes the category name as an argument?

For example, let's say I want to create a generic QML object to support
tracing.  I'd like to give it a category property and a single trace
method, so that I can create separate QML objects for different categories
without having to write any additional C++ code.  I don't see how I can do
that with the current API, as it only supports immortal static
QLoggingCategory objects.

The proposal:

1. Add a destructor to QLoggingCategory that removes it from
QLoggingPrivate's _registeredCategories data member.

This may require changing the type of _registeredCategories from
QList<QLoggingCategory*> to something like QSet<QLoggingCategory*>.

To avoid excess work during shutdown it would make sense to add a flag that
disables updates to _registeredCategories.  Initially this flag would allow
updates.  Upon shutdown (e.g. once QCoreApplication::quit is called) that
flag would be flipped to disable updates.

2. Get rid of the QT_LOG_CATEGORY macro and expose QLogging category
directly.  Let users write code like:

// using a static QLoggingCategory

static QLoggingCategory NETWORK("yoyodyne.network");
...
qCDebug(NETWORK) << "received" << n << "packets";

// using a non-static QLoggingCategory

class ScopedTracer {
public:
    ScopedTracer(const char * cat, const char * what);
    ~ScopedTracer();
private:
    QLoggingCategory _category;
    const char * what;
};

ScopedTracer::ScopedTracer(const char * cat, const char * what)
: _category(cat), _what(what) {
    qCDebug(_category) << "ENTER" << _what;
}

ScopedTracer::~ScopedTracer() {
    qCDebug(_category) << "LEAVE" << _what;
}

...

void myFunction()
{
    ScopedTracer tracer("yoyodyne.myFunction");
    ...
}

// An object for QML

class Tracer : public QObject {
    Q_OBJECT
public:
    Tracer(QObject * parent = 0);
    ~Tracer();
    Q_PROPERTY(QString category WRITE setCategory);
    Q_INVOKABLE void trace(const QString& msg) const;
    void setCategory(const QString& category);
private:
    QLoggingCategory * _category;
};

Tracer::Tracer(QObject * parent)
: QObject(parent), _category(0) {
}

Tracer::~Tracer() {
    delete _category;
}

void Tracer::setCategory(const QString& category) {
    delete _category;
    _category = new QLoggingCategory(category);
}

void Tracer::trace(const QString& msg) const {
    if (!_category) return;
    qCDebug(*_category) << msg;
}
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.qt-project.org/pipermail/development/attachments/20120423/e6801c89/attachment.html>