gtk-rs / gtk

DEPRECATED, use https://github.com/gtk-rs/gtk3-rs repository instead!
https://gtk-rs.org/
MIT License
1.25k stars 82 forks source link

Remove length argument from SelectionData::set_text #477

Closed Susurrus closed 7 years ago

Susurrus commented 7 years ago

The len argument should be inferred from the str argument

GuillaumeGomez commented 7 years ago

Agreed. However, I'm not sure how we could automate this easily. Just checking if a const char* is followed by an integer variable maybe?

Susurrus commented 7 years ago

I think if a pointer of some type was followed by some integer type that was named len, length, or *_length, that would probably cover it. At least the following functions could be fixed by this check (note I didn't do an exhaustive search):

I'm wondering if this information can be automatically parsed from the docs in a more remote way. If you look at the HTML GTK docs for things you see they annotate pointers that are arrays with an [array length=VARIABLE_NAME] annotation. If we could get that in machine-readable form, like in the gir-files, this would all be trivial. Maybe this is something to bring up during the GTK conference next week?

EPashkin commented 7 years ago

gtk_css_provider_load_from_data already says array length="1" gtk_editable_insert_text is just string that not ends with \0 gtk_selection_data_set_text (not get) is same gtk_selection_data_set says array length="3" I think that it be good if it annotated in .gir (but with another attribute) Until then we can do autodetect but need way do disable replacing concrete function in config.

EPashkin commented 7 years ago

Before I thought that it will done by manual adding to concrete function after https://github.com/gtk-rs/gir/issues/264 but maybe I will be good in automatic way

EPashkin commented 7 years ago

Related issue for automatization https://github.com/gtk-rs/gir/issues/376

GuillaumeGomez commented 7 years ago

It'll require to check all these functions really carefully but it's a big improvement if we're able to do it! :)

sdroege commented 7 years ago

I think most of the ones here are not covered because of either 1) unsupported CArray type (fundamental types e.g. are not supported, only pointers). Next on my list 2) missing array-length annotation in GTK (gtk_selection_data_set_text for example, someone please check all of them and file bugs)

sdroege commented 7 years ago

https://bugzilla.gnome.org/show_bug.cgi?id=784022 IMHO we should just patch the GTK .gir file once that is merged.

For fundamental types in arrays, see https://github.com/gtk-rs/gir/issues/388

EPashkin commented 7 years ago

Also don't see this as bug, as it not array and length can be -1

sdroege commented 7 years ago

It is unsafe because you could now provide a length that is bigger than the actual length of the string

EPashkin commented 7 years ago

Normally we replace these functions with manual .len() and thus remove unsafety. If you do something like array_len config we can use it to generate same way

sdroege commented 7 years ago

It's generally unsafe the way how it is in Pango and others right now as especially runtime generated bindings will also expose the length and have no easy way of "configuration" to override this. It's really something that has to be fixed at the gobject-introspection / C library level.

Also these are all actually arrays. "Strings" are arrays in C, and here everywhere defined as [u8] with the additional constraint that they are valid UTF8 (basically just how String in Rust is Vec with the same constraint). And the lengths given here are all in bytes, not characters, so consistent with the [u8] (aka char *) definition.

sdroege commented 7 years ago

See https://github.com/gtk-rs/gir/pull/387#issuecomment-310350093 . Needs manual bindings for these functions that take a string+length, or return a string+length