Skip to content

MSVC: Fix SID - #4505

Merged
tresf merged 3 commits into
LMMS:masterfrom
lukas-w:msvc/sid
Jan 7, 2019
Merged

tresf merged 3 commits into
LMMS:masterfrom
lukas-w:msvc/sid

Conversation

@lukas-w

@lukas-w lukas-w commented Jul 28, 2018

Copy link
Copy Markdown
Member

Fixes SID for MSVC by using the provided working buffer instead of a local one to avoid the use of VLA. Changed the loop to go backwards to avoid overwriting data in the short-to-float conversion. Hope this logic is correct (it appears to sound right).

Use the provided working buffer instead of a local one to avoid use of VLA
@lukas-w lukas-w mentioned this pull request Jul 28, 2018
13 of 15 tasks
@PhysSong

PhysSong commented Aug 9, 2018

Copy link
Copy Markdown
Member

Nice trick! However, I think you should add a comment that describes the reason for your change.

@tresf

tresf commented Aug 27, 2018

Copy link
Copy Markdown
Member

I think you should add a comment that describes the reason for your change.

They were obvious, so I just added them. Hope you don't mind @lukas-w :)

@tresf

tresf commented Jan 7, 2019

Copy link
Copy Markdown
Member

@lukas-w the error from Circle-CI is:

>>>>> linuxdeployqt
linuxdeployqt 5 (commit fc64c50), build 609 built on 2019-01-06 03:55:18 UTC
Please run on a system no newer than the oldest still-supported Ubuntu LTS release.
This is so that the resulting bundle can run on all still-supported releases of Ubuntu.
Exited with code 1

... this seems to be caused by probonopd/linuxdeployqt@fc64c50

@tresf

tresf commented Jan 7, 2019

Copy link
Copy Markdown
Member

I've tested the built-in presets for SID and they sound indistinguishable when compared to the same presets prior to this patch, merging.

@tresf
tresf merged commit a0ace86 into LMMS:master Jan 7, 2019
@PhysSong

PhysSong commented Jan 7, 2019

Copy link
Copy Markdown
Member

... this seems to be caused by probonopd/linuxdeployqt@fc64c50

More precisely, probonopd/linuxdeployqt@b9b57e2 is the right commit. It seems like there were some issues with AppImages created with some build environments. However, I think they should have made linuxdeployqt print a warning when building on a newer environment.

sdasda7777 pushed a commit to sdasda7777/lmms that referenced this pull request Jun 28, 2022
Use the provided working buffer instead of a local one to avoid use of VLA
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

Successfully merging this pull request may close these issues.

3 participants