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

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

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 months ago
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.
@@ -554,4 +569,0 @@
554
-//            dbus_message_iter_get_fixed_array(&sub,&data,&len);
555
-//            return TQCString(data,len);
556
-//         } else {
557
-
SlavekB

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.
MicheleC

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

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

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.
MicheleC

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

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

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
tqdbusmarshall.cpp
@@ -407,6 +420,7 @@ void qFetchStringKeyMapEntry(TQT_DBusDataMap<TQString>& map, DBusMessageIter* it
407 420
     Q_ASSERT(dbus_message_iter_has_next(&itemIter));
408 421
 
409 422
     TQString key = qFetchParameter(&itemIter).toString();
423
+//	tqDebug("qFetchStringKeyMapEntry key : %s", key.latin1());
SlavekB

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

Please, can you remove this unused test output from your patch?
SlavekB commented 5 months ago
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.
deloptes commented 5 months ago
Poster

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
SlavekB commented 5 months ago
Owner

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

All right, it does not matter – I did squash to combine your series of commits into one :smile_cat:
SlavekB deleted branch bug/2925/objectpath 5 months ago
SlavekB added this to the R14.0.6 release milestone 5 months ago
The pull request has been merged.
Sign in to join this conversation.
Loading…
Cancel
Save
There is no content yet.