Skip to content

Fix access mode of files created (#2530) - #2733

Merged
vitaut merged 1 commit into
fmtlib:masterfrom
arogge:fix-2530
Jan 29, 2022
Merged

vitaut merged 1 commit into
fmtlib:masterfrom
arogge:fix-2530

Conversation

@arogge

@arogge arogge commented Jan 26, 2022

Copy link
Copy Markdown
Contributor

The previous fix for this in 4a85db1 was incomplete. The intent was to
mimic what fopen() is doing. As per standard[1] fopen() also sets
S_IWGRP and S_IWOTH and lets the umask handle the rest.

[1] https://pubs.opengroup.org/onlinepubs/9699919799.2018edition/functions/fopen.html

The previous fix for this in 4a85db1 was incomplete. The intent was to
mimic what `fopen()` is doing. As per standard[1] `fopen()` also sets
`S_IWGRP` and `S_IWOTH` and lets the umask handle the rest.

[1] https://pubs.opengroup.org/onlinepubs/9699919799.2018edition/functions/fopen.html
@arogge

arogge commented Jan 26, 2022

Copy link
Copy Markdown
Contributor Author

Do we need automated test for that and if so: how would you like it implemented?

@vitaut

vitaut commented Jan 27, 2022

Copy link
Copy Markdown
Contributor

Thanks for the PR! A test would be nice, please add one to https://github.com/fmtlib/fmt/blob/master/test/os-test.cc (it can be POSIX-specific/disabled on Windows for simplicity).

@vitaut

vitaut commented Jan 29, 2022

Copy link
Copy Markdown
Contributor

Anyway, this is a straightforward fix so I'm merging this in but a PR with a test would be welcome.

@vitaut
vitaut merged commit ae1aaae into fmtlib:master Jan 29, 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.

2 participants