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 515408 - casting int[] to uchar[] breaks .length
casting int[] to uchar[] breaks .length
Status: RESOLVED FIXED
Product: vala
Classification: Core
Component: Arrays
0.3.x
Other All
: Low normal
: ---
Assigned To: Vala maintainers
Vala maintainers
Depends on:
Blocks:
 
 
Reported: 2008-02-09 13:35 UTC by Daniel Svensson
Modified: 2012-08-01 10:15 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Recompute length when casting between array types (1.81 KB, patch)
2012-07-18 20:37 UTC, Simon Werbeck
committed Details | Review

Description Daniel Svensson 2008-02-09 13:35:20 UTC
Please describe the problem:
When casting int[] to uchar[] the length property is set to -1, which leads to problems when vala wants to be smart when passing arrays + length to C-code libs.

This test program is expected to work when this bug has been fixed:

public class Test {
    public static void main(string[] args) {
        int[] tmp = new int[] {1,2,3,4,5};
        weak uchar[] test = (uchar[]) tmp;
        GLib.assert(test.length == tmp.length * (sizeof(int)/sizeof(uchar)));
    }
}


Steps to reproduce:


Actual results:


Expected results:


Does this happen every time?


Other information:
Comment 1 Jürg Billeter 2008-02-09 13:40:32 UTC
Confirming.
Comment 2 Marc-Andre Lureau 2010-01-26 23:48:58 UTC
I am not sure the current behaviour is too bad, it does get the length nowadays (0.7.9)

test = (_tmp2_ = (guchar*) tmp, test_length1 = tmp_length1, test_size = test_length1, _tmp2_);

However, the cast is very straight, there is not type conversion. I am not sure if the bug is about:

- length being -1 after cast: this is solved
- cast being not a type conversion: do we want that?
- if we don't want that, do we want to recompute length the way you proposed? (I don't like the idea so much.. it's too low level)

marking as need info, I would reopen a bug to do type conversion, if it's what we want.
Comment 3 Daniel Svensson 2010-01-27 00:35:56 UTC
Just re-ran the test case, adding the following:

GLib.debug("test.length = %d, tmp.length = %d", test.length, tmp.length);

This yields:
** (process:16533): DEBUG: foo.vala:5: test.length = 5, tmp.length = 5
**
ERROR:/tmp/foo.vala.c:71:test_main: assertion failed: (test_length1 == (tmp_length1 * (sizeof (gint) / sizeof (guchar))))


So it's less broken now (length is initialized), but it's still not set to its correct value.
Comment 4 Marc-Andre Lureau 2010-01-27 01:17:13 UTC
(In reply to comment #3)

> ERROR:/tmp/foo.vala.c:71:test_main: assertion failed: (test_length1 ==
> (tmp_length1 * (sizeof (gint) / sizeof (guchar))))
> 
> 
> So it's less broken now (length is initialized), but it's still not set to its
> correct value.

What is correct? I disagree with your expectation. I would expect type conversion, you would expect array length change.
Comment 5 zarevucky.jiri 2010-01-27 01:25:03 UTC
Is type conversion actually of any use there? My vote goes for array length change, because IMO it's a typecast of the array as a whole, not a conversion of it's elements.
Comment 6 Daniel Svensson 2010-02-09 16:37:09 UTC
Marc-Andre Lureau: If I have (In reply to comment #4)
> > So it's less broken now (length is initialized), but it's still not set to its
> > correct value.
> 
> What is correct? I disagree with your expectation. I would expect type
> conversion, you would expect array length change.

Say that I cast an array of uint8 to an array of uint64, and keep the length like you suggest. Then I will read, and perhaps even write, outside the buffer if I let Vala trust the length that comes along with the variable. Is this recommended behavior according to you?

Correct is when the test passes, otherwise I wouldn't have attached the test in the first place. Unfortunatly I'm not skilled enough to fix it myself, but I stand by the opinion that until this test case passes, Vala is broken.
Comment 7 Daniel Svensson 2010-02-09 16:43:39 UTC
Here is one example affected by this bug:

http://git.xmms.se/?p=abraca.git;a=blob;f=src/components/playlist/playlist_view.vala#l513

And here's another:
http://git.xmms.se/?p=abraca.git;a=blob;f=src/components/filter/filter_view.vala#l500


Both cases of horrible code, and I would be glad to find some better way of doing it, so if you have any suggestions....
Comment 8 zarevucky.jiri 2010-02-09 16:52:58 UTC
Actually, what Marc-Andre suggests (correct me if I'm wrong, but any other interpretation wouldn't make sense) is that the cast should allocate a new array, iterate though the old one and do one-by-one conversion of the content.

something like this:

  int* converted_array = malloc (orig_array_length * sizeof(int));

  for (int i = 0; i < orig_array_length; ++i) {
    converted_array[i] = (int) orig_array[i];
  }

I don't think we want to do this automatically.
Comment 9 Marc-Andre Lureau 2010-02-09 17:11:45 UTC
(In reply to comment #8)
> Actually, what Marc-Andre suggests (correct me if I'm wrong, but any other
> interpretation wouldn't make sense) is that the cast should allocate a new
> array, iterate though the old one and do one-by-one conversion of the content.

yes

> I don't think we want to do this automatically.

maybe not

Perhaps the cast of array of different inner type should be forbidden anyway.
Comment 10 Daniel Svensson 2010-02-09 17:39:57 UTC
How would (In reply to comment #9)
> > I don't think we want to do this automatically.
> 
> maybe not
> 
> Perhaps the cast of array of different inner type should be forbidden anyway.

How would I pass data in drag-n-drop situations without this then?
Comment 11 Marc-Andre Lureau 2010-02-09 17:53:46 UTC
(In reply to comment #10)
> > Perhaps the cast of array of different inner type should be forbidden anyway.
> 

I meant different "inner type size", sorry. But that's not an idea I would like - it's too low-level.

> How would I pass data in drag-n-drop situations without this then?

Create an array of desired types, and loop over all the src array values, with a cast?

However, I still think it's the best, to create a new array and cast each value separately, keeping original array size.

That would be consistent with other casts (such as Value to simple type) and be high level.
Comment 12 zarevucky.jiri 2010-02-09 18:41:40 UTC
(In reply to comment #11)

Value is a special case.
It is actually consistent now, since it doesn't work, just like any other conversion between arbitrary struct types. :))

No, I'm not joking. At the moment, you can't even cast two exactly same struct types. gcc will spit an error, which should probably be a separate bug report.

Simply recomputing length would be consistent with what you'd expect from a cast between structs. I completely disagree with an argument "it is too low level". It is perfectly the behavior one would expect from it. Consistent with both structure casts and pointer casts. Not to say there wouldn't be any alternative if you wanted a low-level cast.

My proposal:

Fix the casts as low-level.
Implement a generic method for arrays for element conversion.
E.g.

int[] arr1;
uint8[] arr2 = arr1.convert<uint8> ();

That would IMO make much more sense.
Comment 13 Daniel Svensson 2010-03-06 17:14:17 UTC
(In reply to comment #11)
> However, I still think it's the best, to create a new array and cast each value
> separately, keeping original array size.


Ok, so in this case I have a list of int32, and GtkSelectionData has this signature:

void     gtk_selection_data_set      (GtkSelectionData     *selection_data,
                                      GdkAtom               type,
                                      gint                  format,
                                      const guchar         *data,
                                      gint                  length);


I can't really loop over my list of int32's and cast them to uchar.
Comment 14 Marc-Andre Lureau 2010-03-07 11:23:56 UTC
(In reply to comment #13)
> (In reply to comment #11)

> I can't really loop over my list of int32's and cast them to uchar.

then I would cast the pointer if you want to bypass conversion, and frankly I wonder why it does not take a pointer.

Documentation says:
data pointer to the data (will be copied)
length of the data
Comment 15 Tobias Mueller 2010-04-22 21:09:32 UTC
(In reply to comment #2)
> marking as need info, I would reopen a bug to do type conversion, if it's what
> we want.
So, any decision on this? I'm a bit confused by the discussion, so Marc-Andre, could you clear up?
Comment 16 Marc-Andre Lureau 2010-04-22 21:16:19 UTC
(In reply to comment #15)
> (In reply to comment #2)
> > marking as need info, I would reopen a bug to do type conversion, if it's what
> > we want.
> So, any decision on this? I'm a bit confused by the discussion, so Marc-Andre,
> could you clear up?

the last proposal from Jiří makes sense.
Comment 17 Tobias Mueller 2010-06-08 09:22:11 UTC
So I'm reopening then since I don't see any open question.
Comment 18 Simon Werbeck 2012-07-18 20:37:10 UTC
Created attachment 219156 [details] [review]
Recompute length when casting between array types

Partially fixes bug 515408
Comment 19 Jürg Billeter 2012-08-01 10:15:30 UTC
Thanks for the patch. I agree that this is the right fix for this bug. Adding a higher level conversion function should be tracked in a separate bug/enhancement report if desired. For future patches, please follow the Vala coding style.

commit d5f417c12f02d56bc377aaacc5e7b299525586cb
Author: Simon Werbeck <simon.werbeck@gmail.com>
Date:   Wed Jul 18 22:11:20 2012 +0200

    Recompute length when casting between array types
    
    This patch makes casting, e.g., from int[] to uchar[] work as expected
    by adjusting the array length field.
    
    Fixes bug 515408.