After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 755731 - Atspi Text Function Fixes
Atspi Text Function Fixes
Status: RESOLVED FIXED
Product: at-spi
Classification: Platform
Component: at-spi2-core
unspecified
Other Linux
: Normal normal
: ---
Assigned To: At-spi maintainer(s)
At-spi maintainer(s)
Depends on:
Blocks:
 
 
Reported: 2015-09-28 13:03 UTC by Patryk Kaczmarek
Modified: 2015-10-13 14:12 UTC
See Also:
GNOME target: ---
GNOME version: ---

Attachments
Fixed atspi_text (3.10 KB, patch)
2015-09-28 13:03 UTC, Patryk Kaczmarek 		committed Details | Review

Description Patryk Kaczmarek 2015-09-28 13:03:52 UTC 
Created attachment 312301 [details] [review]
Fixed atspi_text

Fixed atspi_text_ functions:

* atspi_text_get_text_attribute_value$
    Fixed dbus signature in _atspi_dbus_call function$
    and add missing argument for string.$

* atspi_text_get_default_attributes$
     Receiving return value by reference from hash table$
Comment 1 Mike Gorse 2015-10-13 00:38:16 UTC 
Why this change?

@@ -349,7 +353,7 @@ atspi_text_get_default_attributes (AtspiText *obj, GError **error)
     g_return_val_if_fail (obj != NULL, NULL);
 
   reply = _atspi_dbus_call_partial (obj, atspi_interface_text, "GetDefaultAttributes", error, "");
-  return _atspi_dbus_return_hash_from_message (reply);
+  return g_hash_table_ref (_atspi_dbus_return_hash_from_message (reply));
 }

It should not be needed, since _atspi_dbus_return_hash_from_message ultimately calls g_hash_table_new_full, which returns a hashtable with a ref count of 1, and we don't keep the hash around, so I believe that that change would cause the return value never to be freed even after the caller dereferences it.

I've committed your patch, minus that change. I'll leave the bug open for now, since perhaps you added the ref for a reason.
Comment 2 Mike Gorse 2015-10-13 00:53:32 UTC 
Comment on attachment 312301 [details] [review]
Fixed atspi_text

Committed, minus the change that I had a question about (see last comment).
master: 878684
gnome-3-18: 703990

Thanks for the patches.
Comment 3 Patryk Kaczmarek 2015-10-13 09:44:37 UTC 
Yes you are definitely right, that line is not needed.

Thank you for the review.
Comment 4 Mike Gorse 2015-10-13 14:12:46 UTC 
Okay; closing.