GNOME Bugzilla – Bug 688731
Strings with length argument should be merged into one argument
Last modified: 2018-01-27 11:52:04 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).
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)
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.
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.
(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.
(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.
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.
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.
-- 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.