This is with git master, 049a77952370fcad5f6fbdab08dd84d89f40151a.

Still a problem with 75359c3cab2e979d087ce0a19735bf86add33b42.

We can't auto-null terminate arrays before calling methods. What we can do is null-terminating the constant arrays declared in vala.

(In reply to comment #3)
> What we can do is
> null-terminating the constant arrays declared in vala.

That’s what I was suggesting. :-)

Unfortunately, I don't think we can add a NULL to the constant. Adding it will increase G_N_ELEMENTS, and that would break applications that expose such constants in their public API.

(In reply to comment #5)
> Unfortunately, I don't think we can add a NULL to the constant. Adding it will
> increase G_N_ELEMENTS, and that would break applications that expose such
> constants in their public API.

You could change the code which generates G_N_ELEMENTS() calls to use G_N_ELEMENTS()-1 instead; or something similar.

Still breaks C usage and existing vala applications for public constants, it's not doable.
Perhaps you better var copy = your_array; and use take_boxed.

(In reply to comment #7)
> Perhaps you better var copy = your_array; and use take_boxed.

Why can’t Vala do that automatically? Instead of generating the following code in my_class_get_property():

	case TPF_PERSONA_LINKABLE_PROPERTIES:
	{
		int length;
		g_value_set_boxed (value, folks_persona_get_linkable_properties ((FolksPersona*) self, &length));
	}

it could generate this:

	case TPF_PERSONA_LINKABLE_PROPERTIES:
	{
		gchar **normal_array;
		GStrv *null_terminated_array;
		int length;
		normal_array = folks_persona_get_linkable_properties ((FolksPersona*) self, &length);
		null_terminated_array = _vala_array_dup1 (normal_array, length);
		g_value_take_boxed (value, null_terminated_array);
	}

In any case it needs to do something — whether by disallowing const arrays to be returned from property getters, or (preferably) outputting the code above. At the moment the code Vala generates is broken, and the programmer is given no warning of this.

1) There's no reason to disallow returning const arrays from properties, you are only seeing your own problem here and forgetting that arrays can be non-null terminated only because you're using a function that expects a null terminated array.
2) Vala can't (and won't because we try to avoid subtle copies of arrays) determine if an array is null terminated because of the nature of C.

This would be a WONTFIX, but I don't close as maybe somebody has a good solution.

(In reply to comment #9)
> 1) There's no reason to disallow returning const arrays from properties, you
> are only seeing your own problem here and forgetting that arrays can be
> non-null terminated only because you're using a function that expects a null
> terminated array.

String arrays (of type GStrv) are defined by GLib to be NULL-terminated, so if Vala chooses to generate GStrv code for a string[] property, it should ensure that values returned by that property’s getter are NULL-terminated — either by adding the NULL terminator, or by throwing a compiler error if the programmer tries to return a non-NULL-terminated string array.

> 2) Vala can't (and won't because we try to avoid subtle copies of arrays)
> determine if an array is null terminated because of the nature of C.

You can’t avoid copying the array: g_value_set_boxed() always takes a copy. By using the g_value_take_boxed() suggestion in comment #8 you’re simply moving the copy into the generated C code, rather than doing it inside GLib.

> This would be a WONTFIX, but I don't close as maybe somebody has a good
> solution.

Will my suggestion from comment #8 not work? :-)

(In reply to comment #10)
> String arrays (of type GStrv) are defined by GLib to be NULL-terminated, so if
> Vala chooses to generate GStrv code for a string[] property, it should ensure
> that values returned by that property’s getter are NULL-terminated — either by
> adding the NULL terminator, or by throwing a compiler error if the programmer
> tries to return a non-NULL-terminated string array.

True that, we can forbid returning const arrays defined in vala from within properties. Then you have to handle that by yourself.

> You can’t avoid copying the array: g_value_set_boxed() always takes a copy. By
> using the g_value_take_boxed() suggestion in comment #8 you’re simply moving
> the copy into the generated C code, rather than doing it inside GLib.
> 
> Will my suggestion from comment #8 not work? :-)

Yes, use take_boxed manually :-) As said, vala can't decide that automagically.

By take_boxed manually I mean having an owned getter:

public string[] foobar {
    owned get { return this._foobar; }
}

Maybe something like this, or owned property. Using owned will create a copy for you.

(In reply to comment #11)
> (In reply to comment #10)
> > Will my suggestion from comment #8 not work? :-)
> 
> Yes, use take_boxed manually :-) As said, vala can't decide that automagically.

Why can’t Vala decide that automagically? It knows that the array is const and not NULL-terminated, and it knows that the getter needs to return a NULL-terminated array. What else does it need to know?

(In reply to comment #12)
> public string[] foobar {
>     owned get { return this._foobar; }
> }
> 
> Maybe something like this, or owned property. Using owned will create a copy
> for you.

Changing it to be an owned getter would break folks’ API, so we can’t do that.

(In reply to comment #13)
> Why can’t Vala decide that automagically? It knows that the array is const and
> not NULL-terminated, and it knows that the getter needs to return a
> NULL-terminated array. What else does it need to know?

Wouldn't doing a copy with an unowned getter leak?

> Changing it to be an owned getter would break folks’ API, so we can’t do that.

Keep a copy of the array somewhere and return it.

(In reply to comment #14)
> (In reply to comment #13)
> > Why can’t Vala decide that automagically? It knows that the array is const and
> > not NULL-terminated, and it knows that the getter needs to return a
> > NULL-terminated array. What else does it need to know?
> 
> Wouldn't doing a copy with an unowned getter leak?

Not if you pass it to g_value_take_boxed() instead of g_value_set_boxed().

> > Changing it to be an owned getter would break folks’ API, so we can’t do that.
> 
> Keep a copy of the array somewhere and return it.

That would work, but yeuch.

(In reply to comment #15)
> Not if you pass it to g_value_take_boxed() instead of g_value_set_boxed().

When is it freed then?

(In reply to comment #16)
> (In reply to comment #15)
> > Not if you pass it to g_value_take_boxed() instead of g_value_set_boxed().
> 
> When is it freed then?

When the GValue is unset, same as usual. Take a look at the code for g_value_[take|set]_boxed() in GLib’s gboxed.c. The only difference between them is that g_value_set_boxed() calls g_boxed_copy() on the data it’s passed. For a string array this equates to g_strdupv().

I have mixed two problems here, my bad: the getter and the get_property. In get_property, yes, it's of course possible to copy the array manually instead of letting set_boxed doing it.
The other problem is the getter: there you must return an unowned reference and vala can't do anything about that, other than warning about returning a non-null terminated array.
If we fix get_property, it means we'll have two different behaviors though.

(In reply to comment #18)
> The other problem is the getter: there you must return an unowned reference and
> vala can't do anything about that, other than warning about returning a
> non-null terminated array.

The getter has an integer out parameter which returns the length.
e.g.:

gchar** folks_persona_get_linkable_properties (FolksPersona* self, int* result_length1);

It's ok then, it should be safe to do the copy in get_property.

-- 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/vala/issues/316.