Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Wrong offset in MIDIPacketList struct #374

Closed
1 task done
davidgiga1993 opened this issue May 1, 2019 · 7 comments
Closed
1 task done

Wrong offset in MIDIPacketList struct #374

davidgiga1993 opened this issue May 1, 2019 · 7 comments

Comments

@davidgiga1993
Copy link

davidgiga1993 commented May 1, 2019

Issue details

The struct definition for MIDIPacketList is wrong and only seems to work by accident on 32bit platforms.
When calling getPacket() on a MIDIPacketList struct the returned packet is off by 4 byte on a 64bit platform.

Example:
On 32bit

  • Pointer of list: 6106767000
  • Pointer of getPacket().getHandle(): 6106767004

On 64bit

  • Pointer of list: 6106767000
  • Pointer of getPacket().getHandle(): 6106767008
    Therefore all the data of the MIDIPacket is invalid on 64bit platforms.

The issue is that the packet is not referenced by ref in the original header - therefore the offset is always 4 byte (the numPackets field)

Reproduction steps/code

  • Receive midi data using CoreMidi
  • The length of any midi packet is way too long on 64bit platforms which shows the off by 4.

Configuration

Build Tools:

  • Gradle plugin

Versions:

  • Robovm: 2.3.7-SNAPSHOT
  • XCode: 10.2.1
  • JDK: 1.8.0_181

Build Targets:
iPhone 8 (64 bit)

Workaround

As of now I'm reading the packet with an hardcoded 4 byte offset but that's pretty ugly..

long listPointer = pktlist.getHandle();
MIDIPacket packet = MIDIPacket.toStruct(MIDIPacket.class, listPointer + 4);
@dkimitsa
Copy link
Contributor

dkimitsa commented May 1, 2019

MIDIPacket uses non native struct member packing (#pragma pack(push, 4)), this is not supported by robovm itself. so all data gots natural alignment, which is 8 byte in case of LP64, will check if it hard to tweak struct member compiler a little to support this.

@davidgiga1993
Copy link
Author

I see, thanks for the feedback! As a short term solution: Do you know how I could workaround this issue when creating a MIDIPacketList struct? The workaround I mentioned only applies when reading but not when creating this type.

@dkimitsa
Copy link
Contributor

dkimitsa commented May 1, 2019

you can try manually build a structure in allocated memory (with VM.allocateMemory), and then creating java object for needed memory using MIDIPacket.toStruct(

@florianf
Copy link
Collaborator

@dkimitsa is this also fixed via #375?

@dkimitsa
Copy link
Contributor

hi @florianf, no it requires packed struct support in compiler. I will deliver fix as another PR

@dkimitsa
Copy link
Contributor

hi @davidgiga1993 , sadly it took more than month since reported. but there is a support for packed structs in #378 . Could you please test it and report any issues. Thanks

@dkimitsa
Copy link
Contributor

@florianf @Tom-Ski it was already merged to master, can be closed

@Tom-Ski Tom-Ski closed this as completed Aug 19, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants