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

Throw SurgeError when configuration.xml is not found #251

Merged
merged 1 commit into from
Jan 10, 2019
Merged

Throw SurgeError when configuration.xml is not found #251

merged 1 commit into from
Jan 10, 2019

Conversation

jarkkojs
Copy link
Collaborator

Add a custom exception class SurgeError that is used to fail SurgeStorage constructor when configuration.xml is not found.

Not a perfect fix for #250 but exception is better than a crash for the VST host.

@jarkkojs
Copy link
Collaborator Author

After this fix Renoise at least fails cleanly and does not eat all memory. This commit really helps to continue debugging with Renoise because you don't have to always wait that it eats all your memory and crashes (takes a few minutes). I really hope it gets pulled.

@jarkkojs
Copy link
Collaborator Author

image

Much much nicer end result.

@jarkkojs
Copy link
Collaborator Author

Perhaps SurgeError should just take the message in its constructor because we are only going to just display a message anyway? I.e. I could make it even more dummier. The class is mainly for situations that will anyway end up the plugin being closed so I think that would be perfectly adequate.

@baconpaul
Copy link
Collaborator

I think we probably want a correct error feedback library still since there are other places where we pop a message box and expect to continue operating. My guess is we end up removing this particular throw when we sort everything out. But I agree it is a good interim step on linux so I would be happy to sweep.

Have you built it mac and windows? I don't have time to do that right now. Looks like it should build but there is some stuff which is not in an ifdef.

Add a custom exception class SurgeError that is used to fail SurgeStorage
constructor when configuration.xml is not found. This is needed to avoid
null pointer dereferences because the code after that assumes that the
loading succeeded.

Scope this only for Linux now up until we have a functionality to display
an error dialog for all platforms. This commit prevents Renoise from
exhausting resources and finally crashing in the situation where
configuration.xml is not found.

Signed-off-by: Jarkko Sakkinen <[email protected]>
@baconpaul
Copy link
Collaborator

builds and runs mac; totally standard c++; sweeping with the caveat that I bet this will change again in some regard

@baconpaul baconpaul merged commit 4e63226 into surge-synthesizer:master Jan 10, 2019
@jarkkojs
Copy link
Collaborator Author

@baconpaul, yeah that is why I did not even try to over-engineer the exception class. Something dead simple we can refine as we go :)

@jarkkojs jarkkojs deleted the config-not-found branch January 10, 2019 13:58
baconpaul added a commit to baconpaul/surge that referenced this pull request Jul 10, 2019
Throw SurgeError when configuration.xml is not found

Former-commit-id: 86a7653854921cf4f76fd9e69dfe96622810070f [formerly 4e63226]
Former-commit-id: c948466df3e6f257d80098ff4fa6030454b6e2df
Former-commit-id: 782fb54c33a0c83b8b47d0173cb9219646c3d096
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.

2 participants