Skip to content

Conversation

@ellert
Copy link
Contributor

@ellert ellert commented May 16, 2025

After the issue found in #2508, I found a number of similar issues in other places.

ellert added 3 commits May 16, 2025 19:08
"man 2 open" says:

The argument flags must include one of the following access modes:
O_RDONLY, O_WRONLY, or O_RDWR.

So O_APPEND on its own is not allowed.

The change here is based on "man fdopen":

              ┌──────────────┬───────────────────────────────┐
              │ fopen() mode │ open() flags                  │
              ├──────────────┼───────────────────────────────┤
              │      r       │ O_RDONLY                      │
              ├──────────────┼───────────────────────────────┤
              │      w       │ O_WRONLY | O_CREAT | O_TRUNC  │
              ├──────────────┼───────────────────────────────┤
              │      a       │ O_WRONLY | O_CREAT | O_APPEND │
              ├──────────────┼───────────────────────────────┤
              │      r+      │ O_RDWR                        │
              ├──────────────┼───────────────────────────────┤
              │      w+      │ O_RDWR | O_CREAT | O_TRUNC    │
              ├──────────────┼───────────────────────────────┤
              │      a+      │ O_RDWR | O_CREAT | O_APPEND   │
              └──────────────┴───────────────────────────────┘
Copy link
Member

@amadio amadio left a comment

Choose a reason for hiding this comment

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

Thank you, nice finds. Let's hear also from @abh3 before merging, though. Cheers,

@amadio amadio added this to the 5.8.3 milestone May 17, 2025
@amadio
Copy link
Member

amadio commented May 17, 2025

Thank you, nice finds. Let's hear also from @abh3 before merging, though. Cheers,

Nevermind, I read this on the phone and missed the fact he already approved it. Merging!

@amadio amadio merged commit 1e666f8 into xrootd:master May 17, 2025
11 checks passed
@ellert ellert deleted the fixes branch May 17, 2025 12:08
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.

3 participants