On Wed, Jun 07, 2023 at 11:21:14AM +0200, Christian Brauner wrote:
> On Tue, Jun 06, 2023 at 09:39:47AM +0200, Christoph Hellwig wrote:
> > The only overlap between the block open flags mapped into the fmode_t and
> > other uses of fmode_t are FMODE_READ and FMODE_WRITE. Define a new
>
> and FMODE_EXCL afaict
FMODE_EXCL isn't used outside the block layer and removed in the last
patch.
> > +blk_mode_t file_to_blk_mode(struct file *file)
> > +{
> > + blk_mode_t mode = 0;
> > +
> > + if (file->f_mode & FMODE_READ)
> > + mode |= BLK_OPEN_READ;
> > + if (file->f_mode & FMODE_WRITE)
> > + mode |= BLK_OPEN_WRITE;
> > + if (file->f_mode & FMODE_EXCL)
> > + mode |= BLK_OPEN_EXCL;
> > + if ((file->f_flags & O_ACCMODE) == 3)
>
> I really don't like magic numbers like this.
I don't like them either, but this is just moved around and not new.
> Groan, O_RDONLY being defined as 0 strikes again...
> Becuase of this quirk we internally map
>
> O_RDONLY(0) -> FMODE_READ(1)
> O_WRONLY(1) -> FMODE_WRITE(2)
> O_RDWR(3) -> (FMODE_READ | FMODE_WRITE)
O_RDWR is 2.
> so checking for the raw 3 here is confusing in addition to being a magic
> number as it could give the impression that what's checked here is
> (O_WRONLY | O_RDWR) which doesn't make sense...
Well, that is exactly what we check for. This is a 30-ish year old
quirk only used in the floppy driver.
> So my perference would be in descending order of preference:
>
> (file->f_flags & O_ACCMODE) == (FMODE_READ | FMODE_WRITE)
>
> or while a little less clear but informative enough for people familiar
> with the O_RDONLY quirk:
>
> if ((file->f_flags & O_ACCMODE) == O_ACCMODE)
I don't understand this part. Especially the above doesn't make
any sense as FMODE_READ and FMODE_WRITE are in a completely different
symbol space to O_*, and not a UAPІ but a kernel internal thing that
could be renumbered any time.
