public inbox for [email protected]
 help / color / mirror / Atom feed
From: Linus Torvalds <[email protected]>
To: Jens Axboe <[email protected]>
Cc: Alexander Viro <[email protected]>,
	io-uring <[email protected]>,
	"[email protected]" <[email protected]>
Subject: Re: [GIT PULL] io_uring statx fix for 5.18-rc1
Date: Mon, 21 Mar 2022 16:47:07 -0700	[thread overview]
Message-ID: <CAHk-=wgLwrtFeJ-sitq6f162v1st3GPF7x5BOWJFcn_OHyMYpA@mail.gmail.com> (raw)
In-Reply-To: <[email protected]>

On Fri, Mar 18, 2022 at 2:59 PM Jens Axboe <[email protected]> wrote:
>
> On top of the main io_uring branch, this pull request is for ensuring
> that the filename component of statx is stable after submit. That
> requires a few VFS related changes.

Ugh, I've pulled this, but I hate how it does that

    int getname_statx_lookup_flags(int);

thing with 'int' for both the incoming and outgoing flags.

And I don't say that just because the existing path lookup functions
actually use 'unsigned int', and the code strives to do things like

        unsigned int lookup_flags = LOOKUP_FOLLOW | LOOKUP_DIRECTORY;

So 'int' is ugly, but the _really_ ugly part is how we should have a
separate type for the LOOKUP_xyz flags.

That part isn't new to this change, but this change really highlights
how lacking in type safety that thing is.

The vfs code has a huge pile of different types of 'flags'. Half of
them are various variations of mount flags, I feel, with the whole
MS_xyz -> MNT_xyz thing going on. This is more of that horrid pattern.

At least the mnt code tried to call the variables that keep MNT_xyz
flags 'mnt_flags'. I'm not sure how consistent the code is about it,
but there's _some_ attempt at it.

I do wonder if we should at least try to have a special integer type
for these things that could be checked with sparse (which nobody does)
or at least used as documentation in the function prototypes to show
"this returns a flag of type 'lookup_flag_t' rather than just
'unsigned int' (or worse yet, 'int').

Anyway, I've pulled it, I just wanted to make my reaction to it public
in the hope that some bored vfs person goes "yeah, we should do that"
and works on it.

                Linus

  reply	other threads:[~2022-03-21 23:47 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-03-18 21:59 [GIT PULL] io_uring statx fix for 5.18-rc1 Jens Axboe
2022-03-21 23:47 ` Linus Torvalds [this message]
2022-03-22  0:25 ` pr-tracker-bot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAHk-=wgLwrtFeJ-sitq6f162v1st3GPF7x5BOWJFcn_OHyMYpA@mail.gmail.com' \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox