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 779456 - Make g_utf8_make_valid optionally take a length
Make g_utf8_make_valid optionally take a length
Status: RESOLVED FIXED
Product: glib
Classification: Platform
Component: general
unspecified
Other Linux
: Normal normal
: ---
Assigned To: gtkdev
gtkdev
Depends on:
Blocks:
 
 
Reported: 2017-03-02 08:15 UTC by Paolo Borelli
Modified: 2017-03-02 11:30 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
proof of concept patch (6.75 KB, patch)
2017-03-02 08:18 UTC, Paolo Borelli
needs-work Details | Review
Make g_utf8_make_valid optionally take a length (6.83 KB, patch)
2017-03-02 09:47 UTC, Ignacio Casal Quinteiro (nacho)
committed Details | Review

Description Paolo Borelli 2017-03-02 08:15:58 UTC
g_utf8_make_valid was turned into a public API this cycle. However
now that it is public we should make the API more generic, allowing
the caller to specify the length. This is especially useful if
the function is called with a string that has \0 in the middle
or for chunks of a strings that are not nul terminated.
This is also consistent with most of the other utf8 utils.
    
Callers inside glib are updated to the new signature.
Comment 1 Paolo Borelli 2017-03-02 08:18:42 UTC
Created attachment 347033 [details] [review]
proof of concept patch

I quickly threw together a patch since we are so close to the freeze, however I do not have a building checkout right here, so it is not tested
Comment 2 Ignacio Casal Quinteiro (nacho) 2017-03-02 09:24:26 UTC
I checked it builds and make check passes.
Comment 3 Philip Withnall 2017-03-02 09:40:18 UTC
Review of attachment 347033 [details] [review]:

> g_utf8_make_valid optionally take a length

Seems like the commit subject is missing a verb like ‘Make …’.

::: glib/gutf8.c
@@ +1771,3 @@
   string = NULL;
   remainder = str;
+  remaining_bytes = len;

If you’re going to make @len a gssize (which is correct), I think @remaining_bytes and @valid_bytes need to be gsize.
Comment 4 Ignacio Casal Quinteiro (nacho) 2017-03-02 09:47:57 UTC
Created attachment 347040 [details] [review]
Make g_utf8_make_valid optionally take a length

g_utf8_make_valid was turned into a public API this cycle. However
now that it is public we should make the API more generic, allowing
the caller to specify the length. This is especially useful if
the function is called with a string that has \0 in the middle
or for chunks of a strings that are not nul terminated.
This is also consistent with most of the other utf8 utils.

Callers inside glib are updated to the new signature.
Comment 5 Philip Withnall 2017-03-02 10:21:36 UTC
Review of attachment 347040 [details] [review]:

Looks good to me. Needs to be acked by a GLib maintainer though.
Comment 6 Matthias Clasen 2017-03-02 11:17:34 UTC
Ideally, we'd have done this before landing it. But better now than never. I'm ok with landing this.
Comment 7 Matthias Clasen 2017-03-02 11:19:19 UTC
Review of attachment 347040 [details] [review]:

ok then
Comment 8 Matthias Clasen 2017-03-02 11:19:21 UTC
Review of attachment 347040 [details] [review]:

ok then
Comment 9 Ignacio Casal Quinteiro (nacho) 2017-03-02 11:30:09 UTC
Attachment 347040 [details] pushed as f559bc0 - Make g_utf8_make_valid optionally take a length