Re: [pbs-devel] [PATCH proxmox v2 2/2] sys: open directories with O_CLOEXEC

[Dominik Csapak <d.csapak@proxmox.com>]

On 12/2/24 15:01, Fabian Grünbichler wrote:
> one small nit inline, otherwise:
> 
> Reviewed-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
> 
> On November 29, 2024 3:28 pm, Dominik Csapak wrote:
>> so they don't linger around in case of a daemon reload.
>>
>> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
>> ---
>> new in v2
>>   proxmox-sys/src/fd.rs     |  2 +-
>>   proxmox-sys/src/fs/dir.rs | 15 +++++++++------
>>   2 files changed, 10 insertions(+), 7 deletions(-)
>>
>> diff --git a/proxmox-sys/src/fd.rs b/proxmox-sys/src/fd.rs
>> index 8d85bd2e..386e4222 100644
>> --- a/proxmox-sys/src/fd.rs
>> +++ b/proxmox-sys/src/fd.rs
>> @@ -24,7 +24,7 @@ pub fn change_cloexec(fd: RawFd, on: bool) -> Result<(), anyhow::Error> {
>>   }
>>   
>>   pub(crate) fn cwd() -> Result<OwnedFd, nix::Error> {
>> -    open(".", OFlag::O_DIRECTORY, stat::Mode::empty())
>> +    open(".", crate::fs::DIR_FLAGS, stat::Mode::empty())
>>   }
>>   
>>   pub fn open<P>(path: &P, oflag: OFlag, mode: Mode) -> Result<OwnedFd, nix::Error>
>> diff --git a/proxmox-sys/src/fs/dir.rs b/proxmox-sys/src/fs/dir.rs
>> index c903ab87..a093ed99 100644
>> --- a/proxmox-sys/src/fs/dir.rs
>> +++ b/proxmox-sys/src/fs/dir.rs
>> @@ -14,6 +14,9 @@ use proxmox_lang::try_block;
>>   
>>   use crate::fs::{fchown, CreateOptions};
>>   
>> +/// The default [`OFlag`] we want to use when opening directories.
>> +pub(crate) const DIR_FLAGS: OFlag = OFlag::O_DIRECTORY.union(OFlag::O_CLOEXEC);
> 
> nit: I think I'd prefer a plain `|` here (they are the same in the
> bitflags crate, which this is under the hood).
> 

had the same thought at first, but the `BitOr` traits (and i guess all traits) are not
const, so we can't directly to this here.

what would be possible is this:

---
pub(crate) const DIR_FLAGS: OFlag =
     OFlag::from_bits_truncate(OFlag::O_DIRECTORY.bits() | OFlag::O_CLOEXEC.bits());
---

which is IMHO even uglier than using `.union()` ...


_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel