Skip to content

Conversation

@jamesdlow
Copy link
Contributor

Fix sending MidiPacket when MidiPacket has been created using a pointer to a byte[] rather than a byte[] passed as a reference.
MidiPacket.bytes is intiailized to a Array.Empty so it is never null, and so packet.BytePointer is never used.
We therefore need to check for ByteArray.Length being 0.
Midipacket.bytes is never 0 when intiailized to a 0 length byte[] as that constructor checked for 0 length byte[]

…er to a byte[] rather than a byte[] passed as a reference.

MidiPacket.bytes is intiailized to a Array.Empty<byte> so it is never null, and so packet.BytePointer is never used.
We therefore need to check for ByteArray.Length being 0.
Midipacket.bytes is never 0 when intiailized to a 0 length byte[] as that constructor checked for 0 length byte[]
@rolfbjarne
Copy link
Member

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@rolfbjarne
Copy link
Member

@jamesdlow would you be able to create a self-contained test case for this (not using managed-midi)?

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@dalexsoto
Copy link
Member

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@mandel-macaque
Copy link
Contributor

@jamesdlow @rolfbjarne I have requested a rerun for the failing test.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@rolfbjarne rolfbjarne added the do-not-merge Do not merge this pull request label Sep 18, 2023
@rolfbjarne
Copy link
Member

Marking as do-not-merge until we have tests, since this particular piece of code has regressed multiple times already.

@github-actions
Copy link
Contributor

⚠️ Your code has been reformatted. ⚠️

If this is not desired, add the actions-disable-autoformat label, and revert the reformatting commit.

If files unrelated to your change were modified, try reverting the reformatting commit + merging with the target branch (and push those changes).

@rolfbjarne rolfbjarne removed the do-not-merge Do not merge this pull request label Dec 22, 2023
@jamesdlow
Copy link
Contributor Author

@microsoft-github-policy-service agree

@rolfbjarne
Copy link
Member

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@vs-mobiletools-engineering-service2
Copy link
Collaborator

💻 [CI Build] Tests on macOS M1 - Mac Ventura (13.0) passed 💻

All tests on macOS M1 - Mac Ventura (13.0) passed.

Pipeline on Agent
Hash: [PR build]

@vs-mobiletools-engineering-service2
Copy link
Collaborator

💻 [CI Build] Tests on macOS M1 - Mac Big Sur (11.5) passed 💻

All tests on macOS M1 - Mac Big Sur (11.5) passed.

Pipeline on Agent
Hash: [PR build]

@vs-mobiletools-engineering-service2
Copy link
Collaborator

✅ API diff for current PR / commit

Legacy Xamarin (No breaking changes)
  • iOS (no change detected)
  • tvOS (no change detected)
  • watchOS (no change detected)
  • macOS (no change detected)
NET (empty diffs)
  • iOS: (empty diff detected)
  • tvOS: (empty diff detected)
  • MacCatalyst: (empty diff detected)
  • macOS: (empty diff detected)

✅ API diff vs stable

Legacy Xamarin (No breaking changes)
.NET (No breaking changes)
Legacy Xamarin (stable) vs .NET

ℹ️ Generator diff

Generator Diff: vsdrops (html) vsdrops (raw diff) gist (raw diff) - Please review changes)

Pipeline on Agent
Hash: 07191dfb688d6b3f9de324002661f0b353a65e03 [PR build]

@vs-mobiletools-engineering-service2
Copy link
Collaborator

📚 [PR Build] Artifacts 📚

Packages generated

View packages

Pipeline on Agent
Hash: [PR build]

@vs-mobiletools-engineering-service2
Copy link
Collaborator

💻 [CI Build] Windows Integration Tests passed 💻

All Windows Integration Tests passed.

Pipeline on Agent
Hash: 07191dfb688d6b3f9de324002661f0b353a65e03 [PR build]

@vs-mobiletools-engineering-service2
Copy link
Collaborator

🚀 [CI Build] Test results 🚀

Test results

✅ All tests passed on VSTS: simulator tests.

🎉 All 235 tests passed 🎉

Tests counts

✅ bcl: All 69 tests passed. Html Report (VSDrops) Download
✅ cecil: All 1 tests passed. Html Report (VSDrops) Download
✅ dotnettests: All 1 tests passed. Html Report (VSDrops) Download
✅ fsharp: All 7 tests passed. Html Report (VSDrops) Download
✅ framework: All 8 tests passed. Html Report (VSDrops) Download
✅ generator: All 2 tests passed. Html Report (VSDrops) Download
✅ interdependent_binding_projects: All 7 tests passed. Html Report (VSDrops) Download
✅ install_source: All 1 tests passed. Html Report (VSDrops) Download
✅ introspection: All 8 tests passed. Html Report (VSDrops) Download
✅ linker: All 65 tests passed. Html Report (VSDrops) Download
✅ mac_binding_project: All 1 tests passed. Html Report (VSDrops) Download
✅ mmp: All 2 tests passed. Html Report (VSDrops) Download
✅ mononative: All 6 tests passed. Html Report (VSDrops) Download
✅ monotouch: All 41 tests passed. Html Report (VSDrops) Download
✅ msbuild: All 2 tests passed. Html Report (VSDrops) Download
✅ mtouch: All 1 tests passed. Html Report (VSDrops) Download
✅ xammac: All 3 tests passed. Html Report (VSDrops) Download
✅ xcframework: All 8 tests passed. Html Report (VSDrops) Download
✅ xtro: All 2 tests passed. Html Report (VSDrops) Download

Pipeline on Agent
Hash: 07191dfb688d6b3f9de324002661f0b353a65e03 [PR build]

@rolfbjarne rolfbjarne merged commit d9e5206 into dotnet:main Jan 2, 2024
@rolfbjarne
Copy link
Member

/sudo backport release/8.0.1xx

@vs-mobiletools-engineering-service2
Copy link
Collaborator

Backport Job to branch release/8.0.1xx Created! The magic is happening here

@vs-mobiletools-engineering-service2
Copy link
Collaborator

Hooray! Backport succeeded! Please see https://devdiv.visualstudio.com/DevDiv/_build/results?buildId=8863717 for more details.

rolfbjarne pushed a commit that referenced this pull request Jan 4, 2024
…ated using a point. (#19716)

Fix sending MidiPacket when MidiPacket has been created using a pointer to a byte[] rather than a byte[] passed as a reference.
MidiPacket.bytes is intiailized to a Array.Empty<byte> so it is never null, and so packet.BytePointer is never used.
We therefore need to check for ByteArray.Length being 0.
Midipacket.bytes is never 0 when intiailized to a 0 length byte[] as that constructor checked for 0 length byte[]

Backport of #18981


---------

Co-authored-by: James Low <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

community Community contribution ❤

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants