GNOME Bugzilla – Bug 673783
ClutterTextBuffer crashes
Last modified: 2012-05-09 07:38:06 UTC
We have a small pile of backtraces reported against ClutterTextBuffer in Fedora bugzilla: https://bugzilla.redhat.com/attachment.cgi?id=575851 https://bugzilla.redhat.com/attachment.cgi?id=575633 https://bugzilla.redhat.com/attachment.cgi?id=575112 https://bugzilla.redhat.com/attachment.cgi?id=567525 https://bugzilla.redhat.com/attachment.cgi?id=568807 https://bugzilla.redhat.com/attachment.cgi?id=570576 https://bugzilla.redhat.com/attachment.cgi?id=570582 These are somewhat varied - some in clutter_text_buffer_insert_text, some in clutter_text_buffer_delete_text but they are very likely all the same crash - one evidence is that the 7 backtraces above are reporter 1, reporter 2 - three crashes - reporter 3 - three crashes, and both reporters with multiple crashes have crashes in both insert_text and delete_text. One problem you can find in these backtraces is that clutter_text_set_markup() is passing the wrong length to clutter_text_buffer_insert_text() - it needs: - clutter_text_buffer_set_text (get_buffer (self), text, strlen (text)); + clutter_text_buffer_set_text (get_buffer (self), text, -1); but how does that cause these crashes rather than just a read off the end of 'text'? My best theory is that a *previous* call to clutter_text_buffer_set_text() passed in the wrong length *and* had a length that was greater than CLUTTER_TEXT_BUFFER_MAX_SIZE - in that case, the code in clutter_text_buffer_insert_text(): n_bytes = g_utf8_find_prev_char (chars, chars + n_bytes + 1) - chars; n_chars = g_utf8_strlen (chars, n_bytes); gets n_bytes and n_chars out of sync because of the "embedded" null in the string, and then pv->normal_text_chars and pv->normal_text_bytes are out of sync, and these sorts of crashes will occur in subsequent manipulations of the ClutterText object. I'll attach a patch that fixes up clutter-text.c. Possibly clutter_text_buffer_set_text() should have a g_return_if_fail(n_chars <= g_utf8_strlen(chars)) - some efficiency loss from traversing the string again, though.
Created attachment 211646 [details] [review] ClutterText: Fix length passed to clutter_text_buffer_set_text() clutter_text_buffer_set_text() expects a char count, not a byte count, so pass -1 rather than using strlen.
*** Bug 671231 has been marked as a duplicate of this bug. ***
Review of attachment 211646 [details] [review]: looks good to me
Pushed to clutter-1.10 and master Attachment 211646 [details] pushed as bdf4b3a - ClutterText: Fix length passed to clutter_text_buffer_set_text()
Created attachment 211700 [details] [review] Thanks Owen, that looks like the right fix. I spotted another similar problem with the clutter_text_buffer_new_with_text() API. It may have been better to design it to accept a number of characters instead of number of bytes (as documented). But probably too late to change it now. So we should just make it work correctly: Handle clutter_text_buffer_new_with_text() text_len arg correctly * clutter_text_buffer_new_with_text() accepts a byte count as text_len parameter * Convert to character count before passing to clutter_text_buffer_set_text()
*** Bug 673236 has been marked as a duplicate of this bug. ***
*** Bug 675718 has been marked as a duplicate of this bug. ***