Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Mostly cleanup#59

Open
JuhaManninen wants to merge 14 commits into
BrewBuddyOrg:developmentBrewBuddyOrg/BrewBuddy:developmentfrom
JuhaManninen:developmentJuhaManninen/BrewBuddy:developmentCopy head branch name to clipboard
Open

Mostly cleanup#59
JuhaManninen wants to merge 14 commits into
BrewBuddyOrg:developmentBrewBuddyOrg/BrewBuddy:developmentfrom
JuhaManninen:developmentJuhaManninen/BrewBuddy:developmentCopy head branch name to clipboard

Conversation

@JuhaManninen

@JuhaManninen JuhaManninen commented Feb 2, 2020

Copy link
Copy Markdown

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
Copy Markdown

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.

2 participants

Morty Proxy This is a proxified and sanitized view of the page, visit original site.