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

Cleanup Sandboxie-Plus.ini configuration #2234

Merged
merged 10 commits into from
Sep 17, 2022
Merged

Cleanup Sandboxie-Plus.ini configuration #2234

merged 10 commits into from
Sep 17, 2022

Conversation

okrc
Copy link
Contributor

@okrc okrc commented Sep 15, 2022

Fix #2231

Fix #2231
Fix the issue that BoxCollapsedView is not refreshed in time when groups are renamed and deleted.
Fix #2231
When creating groups nested within groups, m_Groups.append ran twice and now removes a value.
Fix #2231
OldValue should be the underlined version, which should be in the same format as m_Groups Value. Otherwise OldValue can never be found in m_Groups.
Fix BoxDisplayOrder not being refreshed in time when renaming and deleting sandbox operations.
Delete cleans up old data when the sandbox is renamed.
I hope to delete or refuse to write Size by default when m_TotalSize is 0. This avoids extra cleanup work.
Fix #2231
@isaak654 isaak654 marked this pull request as ready for review September 15, 2022 09:42
Disable the use of these characters for the Box Group name, which can cause some exceptions.
@DavidXanatos
Copy link
Member

DavidXanatos commented Sep 16, 2022

c19bda4
and
3e406dc
does not seam to be needed the box object being held by a smart pointer will be available until it goes out of scope so name does not need to be cached

Explicitly deleting SizeCache entries is an improvement, but it does not handle the case where a box is deleted manually from the ini or using the old classic ui.
It would be best to add a clean up routine here

if (!Status.IsError() && !theAPI->GetAllBoxes().contains("defaultbox")) {

such that on start/connect once we have a list of all existing boxes we purge the size cache.

I'll add this to the next build

		if (!Status.IsError()) {

			auto AllBoxes = theAPI->GetAllBoxes();

			foreach(const QString & Key, theConf->ListKeys("SizeCache")) {
				if (!AllBoxes.contains(Key.toLower()))
					theConf->DelValue("SizeCache/" + Key);
			}

			if (!AllBoxes.contains("defaultbox")) {
				OnLogMessage(tr("Default sandbox not found; creating: %1").arg("DefaultBox"));
				theAPI->CreateBox("DefaultBox");
			}
		}

@okrc
Copy link
Contributor Author

okrc commented Sep 16, 2022

@DavidXanatos
This PR doesn't do much more with SizeCache.
I will remove all changes to SizeCache.

c19bda4 and 3e406dc does not seam to be needed the box object being held by a smart pointer will be available until it goes out of scope so name does not need to be cached

Explicitly deleting SizeCach

I don't understand here, is that meant?

a8cdb40
is an additional issue. The names are not determined and escaped for these matches.

@okrc
Copy link
Contributor Author

okrc commented Sep 16, 2022

I'll add this to the next build

		if (!Status.IsError()) {

			auto AllBoxes = theAPI->GetAllBoxes();

			foreach(const QString & Key, theConf->ListKeys("SizeCache")) {
				if (!AllBoxes.contains(Key.toLower()))
					theConf->DelValue("SizeCache/" + Key);
			}

			if (!AllBoxes.contains("defaultbox")) {
				OnLogMessage(tr("Default sandbox not found; creating: %1").arg("DefaultBox"));
				theAPI->CreateBox("DefaultBox");
			}
		}

这里还有一些问题,关闭统计后SizeCache依旧存在,但是Value已经不再更新。

There are still some problems here, after closing Count the SizeCache still exists, but the Value is no longer updated.

@DavidXanatos DavidXanatos merged commit f0f3001 into sandboxie-plus:master Sep 17, 2022
@DavidXanatos
Copy link
Member

I merged the changes and fixed a few things that did not seam right, also added an improved handling of the size cache

@isaak654
Copy link
Collaborator

@DavidXanatos In PRs like this one, I would advise in the next future to use the "Squash and merge" type of merging instead of the traditional "Create a merge commit":
GitHub_merge

The first one is more chaotic when there is a certain number of commits in the same PR.

@okrc okrc deleted the okrc-patch-2 branch September 17, 2022 22:56
@okrc
Copy link
Contributor Author

okrc commented Sep 17, 2022

@DavidXanatos
5275277

			foreach(const QString & Key, theConf->ListKeys("SizeCache")) {
				if (!AllBoxes.contains(Key.toLower()))
					theConf->DelValue("SizeCache/" + Key);
			}

When the WatchBoxSize is modified, the handling should change here. For example:

			if (theConf->GetBool("Options/WatchBoxSize", false)) {
				foreach(const QString & Key, theConf->ListKeys("SizeCache")) {
					if (!AllBoxes.contains(Key.toLower()))
						theConf->DelValue("SizeCache/" + Key);
				}
			}
			else {
				theConf->DelValue("SizeCache");
			}

@DavidXanatos
Copy link
Member

I'll go with

			foreach(const QString & Key, theConf->ListKeys("SizeCache")) {
				if (!AllBoxes.contains(Key.toLower()) || !theConf->GetBool("Options/WatchBoxSize", false))
					theConf->DelValue("SizeCache/" + Key);
			}

@isaak654 isaak654 changed the title Cleanup configuration Cleanup Sandboxie-Plus.ini configuration Sep 19, 2022
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.

Sandboxie-Plus.ini keep old information
3 participants