modify the annotation of @dir and @dentry
Signed-off-by: Congjie Zhou <[email protected]>
---
fs/namei.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/fs/namei.c b/fs/namei.c
index 37fb0a8aa..2fd3ba6a4 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -4095,8 +4095,8 @@ SYSCALL_DEFINE3(mknod, const char __user *, filename, umode_t, mode, unsigned, d
/**
* vfs_mkdir - create directory
* @idmap: idmap of the mount the inode was found from
- * @dir: inode of @dentry
- * @dentry: pointer to dentry of the base directory
+ * @dir: inode of the parent directory
+ * @dentry: dentry of the child directory
* @mode: mode of the new directory
*
* Create a directory.
--
2.34.1
The @dentry is actually the dentry of child directory, but the origin annotation indicates it is dentry of parent directory.
The @dir is similar.
On Sat, Jun 15, 2024 at 07:55:28AM +0100, Al Viro wrote:
> It is an inode of _some_ dentry; it's most definitely not that
> of the argument named 'dentry'.
No need to explain it here, the point was that this belongs into a
useful commit message.
On Sat, Jun 15, 2024 at 12:31:49AM -0700, Christoph Hellwig wrote:
> On Sat, Jun 15, 2024 at 07:55:28AM +0100, Al Viro wrote:
> > It is an inode of _some_ dentry; it's most definitely not that
> > of the argument named 'dentry'.
>
> No need to explain it here, the point was that this belongs into a
> useful commit message.
Seeing that fsdevel is archived, it might be worth spelling out,
actually...
Anyway, yes, something like "correct the inline descriptions of
vfs_mkdir() et.al." ought to go into commit message. And it really
ought to cover not just vfs_mkdir() - other similar comments from
the same commit (6521f8917082 "namei: prepare for idmapped mounts")
have similar issues. "Create a device node or file" for
vfs_mknod() probably ought to be "Create a device node or other
special file", while we are at it...
modify the comments of serveral functions in fs/namei.c, including:
1. vfs_create()
2. vfs_mknod()
3. vfs_mkdir()
4. vfs_rmdir()
5. vfs_symlink()
All of them come from the same commit(6521f8917082 "namei: prepare for idmapped mounts")
Signed-off-by: Congjie Zhou <[email protected]>
---
V1: modify the wrong comments of vfs_mkdir()
V2: polish the comments
V3: modify the wrong comments of other functions similar to vfs_mkdir()
fs/namei.c | 24 ++++++++++++------------
1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/fs/namei.c b/fs/namei.c
index 37fb0a8aa..65347dda7 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -3175,9 +3175,9 @@ static inline umode_t vfs_prepare_mode(struct mnt_idmap *idmap,
/**
* vfs_create - create new file
* @idmap: idmap of the mount the inode was found from
- * @dir: inode of @dentry
- * @dentry: pointer to dentry of the base directory
- * @mode: mode of the new file
+ * @dir: inode of the parent directory
+ * @dentry: dentry of the child file
+ * @mode: mode of the child file
* @want_excl: whether the file must not yet exist
*
* Create a new file.
@@ -3968,9 +3968,9 @@ EXPORT_SYMBOL(user_path_create);
/**
* vfs_mknod - create device node or file
* @idmap: idmap of the mount the inode was found from
- * @dir: inode of @dentry
- * @dentry: pointer to dentry of the base directory
- * @mode: mode of the new device node or file
+ * @dir: inode of the parent directory
+ * @dentry: dentry of the child file
+ * @mode: mode of the child device node or file
* @dev: device number of device to create
*
* Create a device node or file.
@@ -4095,8 +4095,8 @@ SYSCALL_DEFINE3(mknod, const char __user *, filename, umode_t, mode, unsigned, d
/**
* vfs_mkdir - create directory
* @idmap: idmap of the mount the inode was found from
- * @dir: inode of @dentry
- * @dentry: pointer to dentry of the base directory
+ * @dir: inode of the parent directory
+ * @dentry: dentry of the child directory
* @mode: mode of the new directory
*
* Create a directory.
@@ -4177,8 +4177,8 @@ SYSCALL_DEFINE2(mkdir, const char __user *, pathname, umode_t, mode)
/**
* vfs_rmdir - remove directory
* @idmap: idmap of the mount the inode was found from
- * @dir: inode of @dentry
- * @dentry: pointer to dentry of the base directory
+ * @dir: inode of the parent directory
+ * @dentry: dentry of the child directory
*
* Remove a directory.
*
@@ -4458,8 +4458,8 @@ SYSCALL_DEFINE1(unlink, const char __user *, pathname)
/**
* vfs_symlink - create symlink
* @idmap: idmap of the mount the inode was found from
- * @dir: inode of @dentry
- * @dentry: pointer to dentry of the base directory
+ * @dir: inode of the parent directory
+ * @dentry: dentry of the child symlink file
* @oldname: name of the file to link to
*
* Create a symlink.
--
2.34.1
On Sat, Jun 15, 2024 at 12:38:32PM +0800, Congjie Zhou wrote:
> modify the annotation of @dir and @dentry
Well, it is clearly obvious that you modify them from the patch. But
why?
On Fri, Jun 14, 2024 at 11:29:38PM -0700, Christoph Hellwig wrote:
> On Sat, Jun 15, 2024 at 12:38:32PM +0800, Congjie Zhou wrote:
> > modify the annotation of @dir and @dentry
>
> Well, it is clearly obvious that you modify them from the patch. But
> why?
Look at the current comment:
* @dir: inode of @dentry
It is an inode of _some_ dentry; it's most definitely not that
of the argument named 'dentry'.
* @dentry: pointer to dentry of the base directory
No. The first thing vfs_mkdir() does is
int error = may_create(idmap, dir, dentry);
if (error)
return error;
Look at may_create(). Starts with
static inline int may_create(struct mnt_idmap *idmap,
struct inode *dir, struct dentry *child)
{
audit_inode_child(dir, child, AUDIT_TYPE_CHILD_CREATE);
if (child->d_inode)
return -EEXIST;
If the last argument (aka. 'dentry' argument of vfs_mkdir()) is currently
referring to *ANY* directory, you get -EEXIST. For a good and simple
reason: it is the dentry of subdirectory to be created. Now, the second
argument of vfs_mkdir() ('dir') is the inode of the parent to be (or base
directory, if you will).
While we are at it, the rest of comments coming from the same commit
suffer similar problems. vfs_create(), vfs_symlink(), et.al.
vfs_unlink() is fine, vfs_rmdir() should match vfs_unlink() (inode of
parent + dentry of victim).