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.

Sign up for GitHub

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

Jump to bottom

Fix-some-crashes #5144

Merged
merged 2 commits into from
Oct 4, 2023
Merged

Fix-some-crashes #5144

merged 2 commits into from
Oct 4, 2023

Conversation

Febbe
Copy link
Collaborator

@Febbe Febbe commented Sep 13, 2023

@Febbe
Removed a race_condition
a8a092f 
 - fixes xournalpp#5126
 - `view->buffer` from `RenderJob` could be nullptr. when deleted by a different thread
Febbe
Febbe commented Sep 14, 2023
View reviewed changes
Copy link
Collaborator Author

@Febbe Febbe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some issues I found while testing.

src/core/model/Document.cpp Outdated Show resolved Hide resolved
src/core/model/Document.cpp
// Todo (cpp20): use <ranges>
for (auto& c : filename) {
if (forbiddenChars.find(c) != std::string::npos) {
c = '_';
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here we must also check, if a character is greater 127 and or any other invisible character like \n/\t

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should go into util/PathUtil, I think.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The thing is, that this function parses the input from a user, we will never need it elsewhere. It also does not work on paths itself, only on filenames. Gtk already does something similar, it just ignores the filename, if wrong.

This was linked to issues Oct 4, 2023
extent.IsValid assertion error after hitting undo several times #5162
Closed
Xournal crashes *sometimes* when trying to copy-paste written notes from one page to another one #5178
Closed
Crash when using undo after scrolling down many pages #5046
Closed
Crash after clicking on "export as PDF" #5126
Closed
@Febbe Febbe merged commit 79f7c8c into xournalpp:release-1.2 Oct 4, 2023
5 checks passed
@bhennion bhennion added this to the v.1.2.2 milestone Oct 7, 2023
@bhennion bhennion mentioned this pull request Oct 7, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bhennion bhennion bhennion approved these changes

Assignees
No one assigned
Labels
priority::high
Projects
None yet
Milestone
v.1.2.2
2 participants
@Febbe @bhennion