dbus-1-tqt does not support array of objectpathkeymaps #1

Merged
SlavekB merged 1 commits from bug/2925/objectpath into master 5 years ago
Collaborator

https://bugs.trinitydesktop.org/show_bug.cgi?id=2925

When trying to parse data with signature a{oa{sa{sv}}} the flow is something like this:

TQT_DBusProxy::sendWithReply
-> TQT_DBusConnection::sendWithReply
-> TQT_DBusMessage::fromDBusMessage(reply)
-> TQT_DBusMarshall::messageToList(message, dmsg)
-> qFetchParameter(&it)
-> TQT_DBusData qFetchMap
-> qFetchStringKeyMapEntry

It turned out that it does not support array of objectpathkeymaps and it was returning a null key, which explains why it was returning always the last entry

https://bugs.trinitydesktop.org/show_bug.cgi?id=2925 When trying to parse data with signature a{oa{sa{sv}}} the flow is something like this: TQT_DBusProxy::sendWithReply -> TQT_DBusConnection::sendWithReply -> TQT_DBusMessage::fromDBusMessage(reply) -> TQT_DBusMarshall::messageToList(message, dmsg) -> qFetchParameter(&it) -> TQT_DBusData qFetchMap -> qFetchStringKeyMapEntry It turned out that it does not support array of objectpathkeymaps and it was returning a null key, which explains why it was returning always the last entry
SlavekB reviewed 5 years ago
SlavekB left a comment
Owner

The code looks good. It does not seem to cause any change to the API / ABI. Please, just a little cleaning.

Note: Please remove the test output as an ammend of an existing commit and then use push with -f.

The code looks good. It does not seem to cause any change to the API / ABI. Please, just a little cleaning. Note: Please remove the test output as an ammend of an existing commit and then use push with -f.
// dbus_message_iter_get_fixed_array(&sub,&data,&len);
// return TQCString(data,len);
// } else {
Owner

Well, it's a good idea to remove this unused code. It looks a little confusing, so it'll be better if it's gone.

Well, it's a good idea to remove this unused code. It looks a little confusing, so it'll be better if it's gone.
Owner

Any idea why that code was commented out? Could it be useful?

Any idea why that code was commented out? Could it be useful?
Owner

I have no idea. This code is here in a comment from the first commit into the Trinity repository. Additionally, obviously if / else / else is not consistent.

I have no idea. This code is here in a comment from the first commit into the Trinity repository. Additionally, obviously if / else / else is not consistent.
Owner

yes, the commented else is obviously confusing. Given that the code was always commented out from the beginning, removing it seems ok 👍

yes, the commented else is obviously confusing. Given that the code was always commented out from the beginning, removing it seems ok :+1:
Poster
Collaborator

This was the reason why I removed it - it seems it was never used or at least not in this life of this piece of software

This was the reason why I removed it - it seems it was never used or at least not in this life of this piece of software
Q_ASSERT(dbus_message_iter_has_next(&itemIter));
TQString key = qFetchParameter(&itemIter).toString();
// tqDebug("qFetchStringKeyMapEntry key : %s", key.latin1());
Owner

Please, can you remove this unused test output from your patch?

Please, can you remove this unused test output from your patch?
Owner

@deloptes, please do the small cleaning as mentioned in the review above.

@deloptes, please do the small cleaning as mentioned in the review above.
Poster
Collaborator

Hi Slavek,
sorry for the multiple commits, I tried for first time the ammend but failed miserably :)

regards

Hi Slavek, sorry for the multiple commits, I tried for first time the ammend but failed miserably :) regards
Owner

All right, it does not matter – I did squash to combine your series of commits into one 😸

All right, it does not matter – I did squash to combine your series of commits into one :smile_cat:
SlavekB closed this pull request 5 years ago
SlavekB deleted branch bug/2925/objectpath 5 years ago
SlavekB added this to the R14.0.6 release milestone 5 years ago
The pull request has been merged as f126c44916.
Sign in to join this conversation.
No reviewers
No Milestone
No Assignees
3 Participants
Notifications
Due Date

No due date set.

Dependencies

No dependencies set.

Reference: TDE/dbus-1-tqt#1
Loading…
There is no content yet.