Messages in this thread | | | From | Damien Le Moal <> | Subject | Re: [PATCH v3 1/2] fs: New zonefs file system | Date | Wed, 25 Dec 2019 08:21:58 +0000 |
| |
On 2019/12/25 16:20, Damien Le Moal wrote: > On 2019/12/25 15:05, Damien Le Moal wrote: >>>> + inode->i_mode = S_IFREG; >>> >>> i_mode &= ~S_IRWXUGO; ? >> >> Yes, indeed that is better. checkpatch.pl does spit out a warning if one >> uses the S_Ixxx macros though. See below. > > Please disregard this comment. checkpatch is fine. For some reasons I > had warnings in the past but they are now gone. So using the macros > instead of the harder to read hard-coded values.
Retracting this... My apologies for the noise.
Checkpatch does complain about the use of symbolic permissions:
WARNING: Symbolic permissions 'S_IRWXUGO' are not preferred. Consider using octal permissions '0777'. #657: FILE: fs/zonefs/super.c:400: + inode->i_mode &= ~S_IRWXUGO;
I do not understand why this would be a problem. I still went ahead and used the macros as I find the code more readable using them. Please let me know if that is not recommended (checking the code, not surprisingly, many FS use these macros).
> >> >>> >>> Note that clearing the mode flags won't prevent programs with an >>> existing writable fd from being able to call write(). I'd imagine that >>> they'd hit EIO pretty fast though, so that might not matter. >>> >>>> + zone->wp = zone->start; >>>> + } else if (zone->cond == BLK_ZONE_COND_READONLY) { >>>> + inode->i_flags |= S_IMMUTABLE; >>>> + inode->i_mode &= ~(0222); /* S_IWUGO */ >>> >>> Might as well just use S_IWUGO directly here? > > Yes, I did in v4. > >> Because checkpatch spits out a warning if I do. I would prefer using the >> macro as I find it much easier to read. Should I just ignore checkpatch >> warning ? > > My mistake. No warnings :) > >
-- Damien Le Moal Western Digital Research
| |