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

Use "w+" mode for opening/creating the unoconv lock file #49

Merged
merged 2 commits into from
Mar 11, 2024

Conversation

jensschuppe
Copy link
Contributor

@jensschuppe jensschuppe commented Nov 23, 2023

If I understand correctly, attempting a file lock for the unoconv lock file should create it if it doesn't exist. However, fopen() uses r+ which doesn't create the file if it's missing.

systopia-reference: 23962

@jensschuppe jensschuppe added bug Something isn't working status:needs review Code needs review and testing labels Nov 23, 2023
@jofranz
Copy link
Member

jofranz commented Dec 8, 2023

If I remember correctly it was intentionally set as read-only as it seems to be created by a touch() command here. Debatable if this is a good place tho

@jensschuppe
Copy link
Contributor Author

Well, r+ is for reading and writing, just as w+ while the latter also truncates the file. I'm not sure that's what we want, but w and w+ creates the file if it doesn't exist, while r+ does not do that, which is the actual problem here …

@bjendres
Copy link
Member

I don't see any harm in the change, provided it was successfully tested on a local and on the typical client's setup.

@jensschuppe
Copy link
Contributor Author

provided it was successfully tested on […] the typical client's setup.

Not yet, so this will have to wait …

@jensschuppe jensschuppe added status:reviewed and tested Code has received thorough review and test and is ready to be committed/merged and removed status:needs review Code needs review and testing needs funding labels Mar 11, 2024
@jensschuppe
Copy link
Contributor Author

This has been tested by @svenow on another environment, so this is ready to merge now.

@jensschuppe jensschuppe merged commit 4877df6 into master Mar 11, 2024
10 checks passed
@jensschuppe jensschuppe added status:fixed The issue has been resolved (usually by committing/merging code) and removed status:reviewed and tested Code has received thorough review and test and is ready to be committed/merged labels Mar 11, 2024
@jensschuppe jensschuppe added this to the 1.0 milestone Mar 11, 2024
@jensschuppe jensschuppe deleted the lockFileMode branch March 11, 2024 10:44
@jensschuppe jensschuppe mentioned this pull request Mar 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working status:fixed The issue has been resolved (usually by committing/merging code)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants