GNOME Bugzilla – Bug 620312
Fix g_settings_[gs]et_strv() API
Last modified: 2010-06-24 22:51:39 UTC
g_settings_get_strv() and g_settings_set_strv() have a length parameter which is mostly useless since arrays are NULL-terminated. Removing them would be more consistent with most GLib and GTK+ API, and would help keeping these functions as simple as possible. GVariant is still using the length parameter for those who need it. Bindings can deal as they wish with arrays length, anyways. If you don't chose this solution, then I'm also attaching a patch to add the needed annotations for bindings to detect the length parameter.
Created attachment 162500 [details] [review] Remove length parameter for g_settings_[gs]et_strv() Length of the array is redundant since it's NULL-terminated. This is not consistent with many GLib and GTK+ functions, and adds complexity with no real gain, while these convenience functions should be kept simple. g_variant_new_strv() and friends still use a length parameter, though.
Created attachment 162501 [details] [review] Annotate g_variant_*_strv() and g_settings_*_strv() Add GObject introspection annotations so that the length parameter is correctly detected. Also specify that it can be a NULL pointer in getters.
Comment on attachment 162501 [details] [review] Annotate g_variant_*_strv() and g_settings_*_strv() not this one.
Comment on attachment 162500 [details] [review] Remove length parameter for g_settings_[gs]et_strv() i committed this (with a slightly reformatted commit message) plus added another patch that allows @value to be NULL in order to set the empty array since you could do this easily by (NULL, 0) using the old API and i would have missed that feature.
thank you for the poke about the API and the patch to fix it.
Created attachment 162540 [details] [review] Annotate g_variant_[gs]et_strv() and g_settings_set_strv() Add GObject introspection annotations so that the length parameter is correctly detected for g_variant get_strv() and g_variant_set_strv(). Also specify that it can be a NULL pointer in g_variant_get_strv(). For g_settings_set_strv(), allow a NULL value to mean empty array.
Thanks for the change! Here's a new patch about annotations, because GVariant still needs them. And g_settings_set_strv() accepts a NULL value, so better annotate this while we're at it.
please file new bugs for these two issues.
> Length of the array is redundant since it's NULL-terminated. That's not quite right; length still is useful. Returning the length saves you a g_strv_length() if you need the length. > This is not consistent with many GLib and GTK+ functions, Actually that's not quite right. Having a length out param is common, e.g. in glib g_bookmark_file_get_applications, g_bookmark_file_get_uris, g_variant_get_strv, g_variant_dup_strv, g_key_file_get_*_list, etc. all return char** _and_ have a length out param.