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 675890 - Passing NULL buffer to clutter_text_set_text() will not reset the buffer and segfault
Passing NULL buffer to clutter_text_set_text() will not reset the buffer and ...
Status: RESOLVED FIXED
Product: clutter
Classification: Platform
Component: ClutterText
git master
Other Linux
: Normal normal
: ---
Assigned To: clutter-maint
clutter-maint
Depends on:
Blocks:
 
 
Reported: 2012-05-11 15:31 UTC by Dominique Bureau
Modified: 2012-05-21 10:35 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Fix for passing NULL buffer to clutter_text_set_text() (663 bytes, patch)
2012-05-11 15:31 UTC, Dominique Bureau
reviewed Details | Review
ClutterText: Cannot pass NULL string to clutter_text_set_text() (1.43 KB, patch)
2012-05-11 17:44 UTC, Dominique Bureau
committed Details | Review

Description Dominique Bureau 2012-05-11 15:31:39 UTC
Created attachment 213874 [details] [review]
Fix for passing NULL buffer to clutter_text_set_text()

As specified in the documentation, passing a NULL pointer to clutter_text_set_text() should behave the same way as passing an empty string "".

This was working as expected previously, but somehow the behaviour changed at some point and creates 2 issues:
- Passing NULL as a buffer will not reset the string
- If the ClutterText is editable, it will segfault in strcmp

I've attached a proposed patch.
Comment 1 Emmanuele Bassi (:ebassi) 2012-05-11 15:56:32 UTC
Review of attachment 213874 [details] [review]:

thanks for the patch.

looks good, with one minor change.

could you also attach a git formatted patch with a commit message, i.e., use git format-patch to generate the patch? you can also use the git-bz tool. this way we can credit you properly, and apply the patch immediately.

thanks again.

::: clutter-git/clutter/clutter-text.c
@@ +4768,3 @@
   if (self->priv->editable)
     {
+      if (strcmp (clutter_text_buffer_get_text (get_buffer (self)), text ? text : "") == 0)

instead of the ternary operator, you can use g_strcmp0(), which is provided by GLib to be NULL-safe:

  g_strcmp0 (clutter_texture_buffer_get_text (get_buffer (self)), text) == 0

this keeps the condition short and readable.
Comment 2 Dominique Bureau 2012-05-11 17:44:14 UTC
Created attachment 213883 [details] [review]
ClutterText: Cannot pass NULL string to clutter_text_set_text()

Passing a NULL buffer to clutter_text_set_text() does not behave the same
way as passing an empty string "" (as specified in the documentation).
This was working as expected previously, but somehow the behaviour changed
at some point and created 2 new issues:
- Passing a NULL pointer will not reset the string
- If the ClutterText is editable, it will segfault in strcmp

Validations have been added to prevent this.
Comment 3 Dominique Bureau 2012-05-11 17:45:28 UTC
Ok, sent this using git-bz. Since it's the first time I use it, let me know if something is off.
Comment 4 Emmanuele Bassi (:ebassi) 2012-05-11 22:29:31 UTC
Review of attachment 213883 [details] [review]:

stellar. thanks for the patch!
Comment 5 Emmanuele Bassi (:ebassi) 2012-05-21 10:35:00 UTC
Attachment 213883 [details] pushed as a2d40fc - ClutterText: Cannot pass NULL string to clutter_text_set_text()