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 688731 - Strings with length argument should be merged into one argument
Strings with length argument should be merged into one argument
Status: RESOLVED OBSOLETE
Product: gjs
Classification: Bindings
Component: general
unspecified
Other Linux
: Normal normal
: ---
Assigned To: gjs-maint
gjs-maint
Depends on:
Blocks:
 
 
Reported: 2012-11-20 15:08 UTC by lamefun
Modified: 2018-01-27 11:52 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
arg: Support strings being cast to UTF8 arrays (1.89 KB, patch)
2012-12-10 19:49 UTC, Jasper St. Pierre (not reading bugmail)
needs-work Details | Review

Description lamefun 2012-11-20 15:08:34 UTC
g_key_file_load_from_data (GKeyFile *key_file, const gchar *data, gsize length, GError *error);

Length is -1 or size of data in 8-bit integers, but since JavaScript stores strings in 16-bit integers, it's completely useless. In GJS it should be GKeyFile.load_from_data(data) instead of GKeyFile.load_from_data(data, length).
Comment 1 Jasper St. Pierre (not reading bugmail) 2012-11-20 15:26:23 UTC
Yeah, I wrote a patch for this a little while ago, but I guess I forgot to attach it.

(Will do when I'm at the machine that has it, just trying to stop Giovanni from getting to it first)
Comment 2 Jasper St. Pierre (not reading bugmail) 2012-12-10 19:49:34 UTC
Created attachment 231199 [details] [review]
arg: Support strings being cast to UTF8 arrays

This is necessary for things like pango_parse_markup and other
such functions that take a string/length pair and use an (array)
annotation to mark it up correctly.
Comment 3 Colin Walters 2012-12-10 20:23:52 UTC
Review of attachment 231199 [details] [review]:

First, let's create a term to describe this.  How about "utf8-maybe-byte-truncated"?

Next, what problem are we solving here?  From IRC discussion, it sounded like there were two:

1) For functions that accept -1, application authors found it confusing that gjs required the length parameter.
2) For functions that don't accept -1, it's actually an evil trap to use string.length because for most of these APIs, the length is in *bytes* not characters, which will end up truncating non-ASCII strings.

Now that we have the problem spelled out better:

pango_parse_markup() is *not* annotated with (array).  Neither is gtk_text_buffer_insert(), or g_key_file_load_from_data().

There is no annotation to describe this at the moment, actually.   

This patch has no tests, and I don't have any idea how it would work.  We're not talking about "arrays of utf8" (that's char**, or GPtrArray*).

Now were we to try to eliminate the length parameter, we have a backwards compatibility problem.  It's a "BAPI" change (binding API).  If we suddenly drop the length parameter, people's scripts doing:

keyfile.load_from_data("[foo]\nbar=true", -1, 0);

would suddenly explode.

I think the best fix at this point is to change any C API which doesn't honor -1 to do so, and just continue passing -1 from gjs.
Comment 4 Jasper St. Pierre (not reading bugmail) 2012-12-10 22:22:23 UTC
(In reply to comment #3)
> pango_parse_markup() is *not* annotated with (array).  Neither is
> gtk_text_buffer_insert(), or g_key_file_load_from_data().

Huh. Apparently I had a local patch to Pango and GKeyFile that I forgot about.

> There is no annotation to describe this at the moment, actually.   
>
> This patch has no tests, and I don't have any idea how it would work.  We're
> not talking about "arrays of utf8" (that's char**, or GPtrArray*).

Gah, I forgot to add them to the commit.

> Now were we to try to eliminate the length parameter, we have a backwards
> compatibility problem.  It's a "BAPI" change (binding API).  If we suddenly
> drop the length parameter, people's scripts doing:
> 
> keyfile.load_from_data("[foo]\nbar=true", -1, 0);
> 
> would suddenly explode.
> 
> I think the best fix at this point is to change any C API which doesn't honor
> -1 to do so, and just continue passing -1 from gjs.

Some of these arguments are unsigned, which means that they can't validly take -1.
Comment 5 Colin Walters 2012-12-11 01:35:03 UTC
(In reply to comment #4)
> 
> Some of these arguments are unsigned, which means that they can't validly take
> -1.

Like g_key_file_load_from_data()?  Yeah...the other alternative is a new API which drops the length.  Obviously it does suck to add new API merely to invoke strlen() though =/

Now I am curious what the rest of your patch with annotations does, but the backcompat problem is quite real.
Comment 6 lamefun 2012-12-27 17:11:59 UTC
It looks like load_from_data functions are to operate on binary data, not on strings. GLib.KeyFile.load_from_data() that accepts binary-buffer or Uint8 arrays and GLib.KeyFile.load_from_string() that accepts strings seems like the best solution for me.
Comment 7 lamefun 2013-01-01 18:26:36 UTC
Also, there's also GtkTextBuffer.set_text(text, len); This one clearly accepts strings, while GKeyFile.load_from_data(data, len) clearly accepts binary data and there's no way to distinguish them with current GObject introspection.
Comment 8 GNOME Infrastructure Team 2018-01-27 11:52:04 UTC
-- GitLab Migration Automatic Message --

This bug has been migrated to GNOME's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/gjs/issues/66.