[Development] QLogger questions and proposal

[Mike Zraly]

On Mon, Apr 23, 2012 at 7:57 PM, Lincoln Ramsay <lincoln.ramsay at nokia.com>wrote:

> On 04/24/2012 04:47 AM, ext Mike Zraly wrote:
> > 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.
>
> Someone else mentioned this (or a similar problem) which prompted me to
> come up with this patch: https://codereview.qt-project.org/23817
>
> The thing is, I don't know that this by itself is enough to solve your
> problem but it certainly works.
>
> > 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.
>
> So I guess for now, don't do that ;)
>


> Second, how can I use qCDebug with a QLoggingCategory object defined
> > inside a class or function which takes the category name as an argument?
>
> Oh, this is the same thing that patch is designed to support.
>
> If you check out the QT_LOG_CATEGORY macro you can see that it just
> creates an object inside a namespace. With that patch you should be able
> to construct such objects wherever and whenever you feel like it and
> when they go away things will be cleaned up appropriately.
>
> I know there was a suggestion at one point that we ditch that macro and
> just have concrete classes declared (and passed to qCDebug). If we do
> that, your use case becomes trivial to support.
>
> > 1. Add a destructor to QLoggingCategory that removes it from
> > QLoggingPrivate's _registeredCategories data member.
>
> See the patch.
>

I did.  Functionally it looks fine.

I'm a little concerned about performance during shutdown if there are a
large number (say 100 or more) of QLoggingCategory objects listed in
_registeredCategories though, as each each call to QList::removeOne will do
a linear scan for the address to remove.  In the worst case where
QLoggingCategory objects are destructed in reverse order of their being
added to _registeredCategories the time for scanning will grow as O(n**2).
 That can be addressed later if it turns out to be a real problem in
practice.


> 2. Get rid of the QT_LOG_CATEGORY macro and expose QLogging category
> > directly.
>
> Yeah... what I said before.
>
> I think there is merit in this idea. We may have to be a little careful
> since people have already started using this code (ie. I'd like to keep
> source compatibility) but I think this is worth a try.


Fair enough.  But if you must keep QT_LOG_CATEGORY redefine it so that you
can still pass QLoggingCategory references or addresses to qCDebug.  The
key to that is using QT_LOG_CATEGORY's first argument as the generated
variable name, then exposing whatever namespace it is in to the code that
calls qCDebug.  I see three options:

1. Skip the namespace altogether, rely on the static keyword to limit the
variable's scope.
2. Use an unnamed namespace for the same effect.
3. Add using directives to expose the QtLogger::QLoggingCategories
namespace.

Options 1 or 2 seem the simplest to me, but there may be reasons for using
the namespace that I have overlooked.

Mike
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.qt-project.org/pipermail/development/attachments/20120424/004135d4/attachment.html>