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

Fix potential index out of bounds error #446

Merged
merged 1 commit into from
Jan 30, 2021
Merged

Fix potential index out of bounds error #446

merged 1 commit into from
Jan 30, 2021

Conversation

kleonc
Copy link
Contributor

@kleonc kleonc commented Jan 29, 2021

In OpenSave.reload_backup_file after the first removal with project_paths.remove(backup_paths.find(backup)) the index returned by backup_paths.find(backup) will no longer point to the project path respective to that backup, project_paths and backup_paths arrays would be "desynchronized". This PR makes both arrays be changed properly when iterating over the backup paths (and shrinked if needed after the loop).

Copy link
Member

@OverloadedOrama OverloadedOrama left a comment

Choose a reason for hiding this comment

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

Looks good! This closes #445 right?

@kleonc
Copy link
Contributor Author

kleonc commented Jan 30, 2021

This closes #445 right?

No, it's unrelated. Just noticed this one after a brief look at the OpenSave.reload_backup_file method when I've got crash described in #445. But error fixed here isn't the cause of #445.

@OverloadedOrama
Copy link
Member

Oh you are right. I tested the GitHub Actions generated build first which doesn't crash and I thought the issue was fixed, my bad. Anyway, thank you!

@OverloadedOrama OverloadedOrama merged commit 499251f into Orama-Interactive:master Jan 30, 2021
@kleonc kleonc deleted the reload_backups_fix branch January 30, 2021 19:15
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