Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Separating notification data from Notification class #7

Closed
wants to merge 1 commit into from
Closed

Separating notification data from Notification class #7

wants to merge 1 commit into from

Conversation

SfietKonstantin
Copy link
Member

This commit introduces a NotificationData class, that is used to carry the
data of a Notification through DBus. It also fixes the inability to compile in
Debug mode.

…fication class

This commit introduces a NotificationData class, that is used to carry the
data of a Notification through DBus. It also fixes the inability to compile in
Debug mode.
@faenil
Copy link
Member

faenil commented Aug 28, 2014

looks sane to me :)

Disclaimer: I've never touched/read anything about the code in this repo before

@SfietKonstantin
Copy link
Member Author

Anyone else could take a look ? @rburchell ?

@rburchell
Copy link
Contributor

What I don't understand is why the change needs to be made. You should probably explain that in the commit message.

I'm also unsure of the effects of the dbus signature change, so I'm not really able to 'sign off' on this, unfortunately.

@SfietKonstantin
Copy link
Member Author

The problem is basically that QObject don't have a copy constructor, so neither do Notification. A QList therefore shouldn't be able to build. (And in fact, in debug mode, the code just don't build). I will add this information to the commit message.

Using a plain old data structure make the code to at least build.

The DBus signature itself actually don't change. What changes is which object is used to perform mangling / demangling to DBus. Previously, it was Notification, and now it is NotificationData.

@thp
Copy link
Contributor

thp commented Jan 29, 2015

Merge conflicts. Please rebase, otherwise we might have to close this one.

matthewvogt added a commit to matthewvogt/nemo-qml-plugin-notifications that referenced this pull request Feb 2, 2015
One particular closed source application generated a repeatable
crash with version 0.0.9, ending in the following stack trace:

 #0  0x42b26808 in __GI_raise (sig=6) at ../nptl/sysdeps/unix/sysv/linux/raise.c:67
 nemomobile#1  0x42b2a590 in __GI_abort () at abort.c:91
 nemomobile#2  0x42b61740 in __libc_message (do_abort=2, fmt=0x42c25e58 "*** glibc detected *** %s: %s: 0x%s ***\n") at ../sysdeps/unix/sysv/linux/libc_fatal.c:201
 nemomobile#3  0x42b6c154 in malloc_printerr (action=3, str=0x42c26064 "free(): invalid pointer", ptr=<optimized out>) at malloc.c:5047
 nemomobile#4  0x4b2ea51c in deallocate (data=<optimized out>) at /usr/include/qt5/QtCore/qarraydata.h:230
 nemomobile#5  ~QString (this=<optimized out>, __in_chrg=<optimized out>) at /usr/include/qt5/QtCore/qstring.h:921
 nemomobile#6  ~QString (this=<optimized out>, __in_chrg=<optimized out>) at /usr/include/nemonotifications-qt5/notification.h:41
 nemomobile#7  ~Notification (this=0x4dd38138, __in_chrg=<optimized out>) at /usr/include/nemonotifications-qt5/notification.h:41
 nemomobile#8  Notification::~Notification (this=0x4dd38138, __in_chrg=<optimized out>) at /usr/include/nemonotifications-qt5/notification.h:41
 nemomobile#9  0x4b543c1c in QtMetaTypePrivate::QMetaTypeFunctionHelper<Notification, true>::Delete (t=<optimized out>) at /usr/include/qt5/QtCore/qmetatype.h:713
 nemomobile#10 0x4243b048 in customTypeDestroyer (where=0x4dd38138, type=<optimized out>) at kernel/qmetatype.cpp:1657
 nemomobile#11 delegate (where=0x4dd38138, this=<synthetic pointer>) at kernel/qmetatype.cpp:1643
 nemomobile#12 switcher<void, {anonymous}::TypeDestroyer> (data=0x4dd38138, type=<optimized out>, logic=<synthetic pointer>) at kernel/qmetatypeswitcher_p.h:83
 nemomobile#13 QMetaType::destroy (type=<optimized out>, data=0x4dd38138) at kernel/qmetatype.cpp:1673
 nemomobile#14 0x42467e88 in (anonymous namespace)::customClear (d=0xbea76ba8) at kernel/qvariant.cpp:886
 nemomobile#15 0x42468290 in ~QVariant (this=0xbea76ba8, __in_chrg=<optimized out>) at kernel/qvariant.cpp:1208
 nemomobile#16 QVariant::~QVariant (this=0xbea76ba8, __in_chrg=<optimized out>) at kernel/qvariant.cpp:1205
 nemomobile#17 0x41f73468 in QDBusArgumentPrivate::createSignature (id=1899) at qdbusargument.cpp:105
 nemomobile#18 0x41f7b1f8 in QDBusMetaType::typeToSignature (type=<optimized out>) at qdbusmetatype.cpp:462
 nemomobile#19 0x41f73940 in beginArray (id=1899, this=0x4f49cbe8) at qdbusmarshaller.cpp:237
 nemomobile#20 QDBusArgument::beginArray (this=0xbea76cd0, id=1899) at qdbusargument.cpp:868
 nemomobile#21 0x4b547014 in operator<< <Notification> (list=..., arg=...) at /usr/include/qt5/QtDBus/qdbusargument.h:264
 nemomobile#22 qDBusMarshallHelper<QList<Notification> > (arg=..., t=0xbea76cb8) at /usr/include/qt5/QtDBus/qdbusmetatype.h:69
 nemomobile#23 0x41f7b964 in QDBusMetaType::marshall (arg=..., id=<optimized out>, data=0xbea76cb8) at qdbusmetatype.cpp:251
 nemomobile#24 0x41f733a8 in QDBusArgumentPrivate::createSignature (id=1900) at qdbusargument.cpp:82
 nemomobile#25 0x41f7b1f8 in QDBusMetaType::typeToSignature (type=<optimized out>) at qdbusmetatype.cpp:462
 #26 0x41f86418 in QDBusPendingCallPrivate::setMetaTypes (this=0x4f456a80, count=1, types=<optimized out>) at qdbuspendingcall.cpp:199
 #27 0x41f884c4 in QDBusPendingReplyData::setMetaTypes (this=0xbea76dcc, count=1, types=0xbea76d88) at qdbuspendingreply.cpp:293
 #28 0x4b548834 in calculateMetaTypes (this=0xbea76dcc) at /usr/include/qt5/QtDBus/qdbuspendingreply.h:195
 #29 assign (call=..., this=0xbea76dcc) at /usr/include/qt5/QtDBus/qdbuspendingreply.h:201
 #30 operator= (call=..., this=0xbea76dcc) at /usr/include/qt5/QtDBus/qdbuspendingreply.h:144
 #31 QDBusPendingReply (call=..., this=0xbea76dcc) at /usr/include/qt5/QtDBus/qdbuspendingreply.h:138
 #32 NotificationManagerProxy::GetNotifications (this=<optimized out>, app_name=...) at notificationmanagerproxy.h:57
 #33 0x4b5412e8 in Notification::notifications () at notification.cpp:665

It may be an error in the application's use of QDBus, but in any case
the problem is not reproducible with the layout change in this commit.
@thp
Copy link
Contributor

thp commented Feb 3, 2015

Superseded by PR #12. Continue the discussion there.

@thp thp closed this Feb 3, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants