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

Mostly cleanup #59

Open
wants to merge 14 commits into
base: development
Choose a base branch
from

Conversation

JuhaManninen
Copy link

@JuhaManninen JuhaManninen commented Feb 2, 2020

My commits clean up the sources and the whole repository.
Reducing compiler messages about unused variables let's you see real problems more easily. There were real problems like uninitialized variables.
I removed all compiled binary files. They should not be under revision control.
I also removed whole unused Orpheus and OSspecific.

There is one commit where I tried to fix potential memory issues:

  • Use "override" and "reintroduce" directives in destructors and methods. Fix visibility.

Some destructors missed "override". Destructor for TBase was redefined as "virtual". The compiler skipped calling it which can be considered a compiler bug. Changing it to "override" triggered a crash in unrelated GUI component code. There are indeed some serious memory management issues. My patch obviously does not change it to "override". The app is as usable as earlier.

Instead of container classes the code uses dynamic arrays and calls SetLength().
Usually FPC debug flags reveal such memory bugs but not now. :(
I added some warnings. Please take a look.

@jortkoopmans
Copy link

Linked to issue #58

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.

None yet

2 participants