[{"id":3679354,"web_url":"http://patchwork.ozlabs.org/comment/3679354/","msgid":"<CAOQ4uxhh1kGnuY7sKTqti12tjFD4t1vCJ0qQrGngU2+0idXtpQ@mail.gmail.com>","list_archive_url":null,"date":"2026-04-20T10:29:12","subject":"Re: [PATCH v2] vfs: replace ints with enum component_type for\n LAST_XXX","submitter":{"id":4703,"url":"http://patchwork.ozlabs.org/api/people/4703/","name":"Amir Goldstein","email":"amir73il@gmail.com"},"content":"On Sun, Apr 19, 2026 at 6:16 PM Jori Koolstra <jkoolstra@xs4all.nl> wrote:\n>\n> Several functions in namei.c take an \"int *type\" parameter, such as\n> filename_parentat(). To know what values this can take you have to find\n> the anonymous struct that defines the LAST_XXX values. I would argue\n> that the readability of the code is improved by making this an explicit\n> type.\n>\n> Signed-off-by: Jori Koolstra <jkoolstra@xs4all.nl>\n> Reviewed-by: Jan Kara <jack@suse.cz>\n>\n> ---\n>\n> v2: move back to LAST_XXX and change int to component_type in\n> fs/smb/server/vfs.c.\n>\n> ---\n>  fs/namei.c            | 41 ++++++++++++++++++++++-------------------\n>  fs/smb/server/vfs.c   |  5 +++--\n>  include/linux/namei.h |  4 ++--\n>  3 files changed, 27 insertions(+), 23 deletions(-)\n>\n> diff --git a/fs/namei.c b/fs/namei.c\n> index 9e5500dad14f..a880454a6415 100644\n> --- a/fs/namei.c\n> +++ b/fs/namei.c\n> @@ -721,15 +721,15 @@ EXPORT_SYMBOL(path_put);\n>\n>  #define EMBEDDED_LEVELS 2\n>  struct nameidata {\n> -       struct path     path;\n> -       struct qstr     last;\n> -       struct path     root;\n> -       struct inode    *inode; /* path.dentry.d_inode */\n> -       unsigned int    flags, state;\n> -       unsigned        seq, next_seq, m_seq, r_seq;\n> -       int             last_type;\n\nPlease update documentation entry for last_type in\nDocumentation/filesystems/path-lookup.rst\n\n> -       unsigned        depth;\n> -       int             total_link_count;\n> +       struct path             path;\n> +       struct qstr             last;\n> +       struct path             root;\n> +       struct inode            *inode; /* path.dentry.d_inode */\n> +       unsigned int            flags, state;\n> +       unsigned                seq, next_seq, m_seq, r_seq;\n> +       enum component_type     last_type;\n\nI really dislike all this white space churn just because the enum name\nis so long. If it was long for a good reason - increasing readability -\nI may have conceded, but I don't think that is the case.\n\nI do not think that 'component_type' is more clear than 'last_type'\nfor a developer passing by reading the code.\nIf anything, the opposite, because 'last_type' or 'type' variables always\nappear in the same function with 'last' variable, so it is more clear WHICH\nobject the type is referring to and matches the LAST_ enum prefix.\n\n> +       unsigned                depth;\n> +       int                     total_link_count;\n>         struct saved {\n>                 struct path link;\n>                 struct delayed_call done;\n> @@ -2221,7 +2221,7 @@ static struct dentry *follow_dotdot(struct nameidata *nd)\n>         return dget(nd->path.dentry);\n>  }\n>\n> -static const char *handle_dots(struct nameidata *nd, int type)\n> +static const char *handle_dots(struct nameidata *nd, enum component_type type)\n>  {\n>         if (type == LAST_DOTDOT) {\n>                 const char *error = NULL;\n> @@ -2869,7 +2869,7 @@ static int path_parentat(struct nameidata *nd, unsigned flags,\n>  /* Note: this does not consume \"name\" */\n>  static int __filename_parentat(int dfd, struct filename *name,\n>                                unsigned int flags, struct path *parent,\n> -                              struct qstr *last, int *type,\n> +                              struct qstr *last, enum component_type *type,\n>                                const struct path *root)\n>  {\n>         int retval;\n> @@ -2894,7 +2894,7 @@ static int __filename_parentat(int dfd, struct filename *name,\n>\n>  static int filename_parentat(int dfd, struct filename *name,\n>                              unsigned int flags, struct path *parent,\n> -                            struct qstr *last, int *type)\n> +                            struct qstr *last, enum component_type *type)\n>  {\n>         return __filename_parentat(dfd, name, flags, parent, last, type, NULL);\n>  }\n> @@ -2963,7 +2963,8 @@ static struct dentry *__start_removing_path(int dfd, struct filename *name,\n>         struct path parent_path __free(path_put) = {};\n>         struct dentry *d;\n>         struct qstr last;\n> -       int type, error;\n> +       enum component_type type;\n> +       int error;\n>\n>         error = filename_parentat(dfd, name, 0, &parent_path, &last, &type);\n>         if (error)\n> @@ -3009,7 +3010,8 @@ struct dentry *kern_path_parent(const char *name, struct path *path)\n>         CLASS(filename_kernel, filename)(name);\n>         struct dentry *d;\n>         struct qstr last;\n> -       int type, error;\n> +       enum component_type type;\n> +       int error;\n>\n>         error = filename_parentat(AT_FDCWD, filename, 0, &parent_path, &last, &type);\n>         if (error)\n> @@ -3057,7 +3059,7 @@ EXPORT_SYMBOL(kern_path);\n>   * @root: pointer to struct path of the base directory\n>   */\n>  int vfs_path_parent_lookup(struct filename *filename, unsigned int flags,\n> -                          struct path *parent, struct qstr *last, int *type,\n> +                          struct path *parent, struct qstr *last, enum component_type *type,\n>                            const struct path *root)\n>  {\n>         return  __filename_parentat(AT_FDCWD, filename, flags, parent, last,\n> @@ -4903,7 +4905,7 @@ static struct dentry *filename_create(int dfd, struct filename *name,\n>         bool want_dir = lookup_flags & LOOKUP_DIRECTORY;\n>         unsigned int reval_flag = lookup_flags & LOOKUP_REVAL;\n>         unsigned int create_flags = LOOKUP_CREATE | LOOKUP_EXCL;\n> -       int type;\n> +       enum component_type type;\n>         int error;\n>\n>         error = filename_parentat(dfd, name, reval_flag, path, &last, &type);\n> @@ -5365,7 +5367,7 @@ int filename_rmdir(int dfd, struct filename *name)\n>         struct dentry *dentry;\n>         struct path path;\n>         struct qstr last;\n> -       int type;\n> +       enum component_type type;\n>         unsigned int lookup_flags = 0;\n>         struct delegated_inode delegated_inode = { };\n>  retry:\n> @@ -5383,6 +5385,7 @@ int filename_rmdir(int dfd, struct filename *name)\n>         case LAST_ROOT:\n>                 error = -EBUSY;\n>                 goto exit2;\n> +       case LAST_NORM: ; // OK\n\nI was not aware that the compiler warns about missing cases for enums -\ncool feature.\nbut that needs to be a break; rather than fallthough the end\nand as a matter of taste, I would put the valid case first - not critical.\n\n>         }\n>\n>         error = mnt_want_write(path.mnt);\n> @@ -5507,7 +5510,7 @@ int filename_unlinkat(int dfd, struct filename *name)\n>         struct dentry *dentry;\n>         struct path path;\n>         struct qstr last;\n> -       int type;\n> +       enum component_type type;\n>         struct inode *inode;\n>         struct delegated_inode delegated_inode = { };\n>         unsigned int lookup_flags = 0;\n> @@ -6074,7 +6077,7 @@ int filename_renameat2(int olddfd, struct filename *from,\n>         struct renamedata rd;\n>         struct path old_path, new_path;\n>         struct qstr old_last, new_last;\n> -       int old_type, new_type;\n> +       enum component_type old_type, new_type;\n>         struct delegated_inode delegated_inode = { };\n>         unsigned int lookup_flags = 0;\n>         bool should_retry = false;\n> diff --git a/fs/smb/server/vfs.c b/fs/smb/server/vfs.c\n> index d08973b288e5..b12d481f5ba9 100644\n> --- a/fs/smb/server/vfs.c\n> +++ b/fs/smb/server/vfs.c\n> @@ -56,7 +56,8 @@ static int ksmbd_vfs_path_lookup(struct ksmbd_share_config *share_conf,\n>  {\n>         struct qstr last;\n>         const struct path *root_share_path = &share_conf->vfs_path;\n> -       int err, type;\n> +       int err;\n> +       enum component_type type;\n>         struct dentry *d;\n>\n>         if (pathname[0] == '\\0') {\n> @@ -668,7 +669,7 @@ int ksmbd_vfs_rename(struct ksmbd_work *work, const struct path *old_path,\n>         struct renamedata rd;\n>         struct ksmbd_share_config *share_conf = work->tcon->share_conf;\n>         struct ksmbd_file *parent_fp;\n> -       int new_type;\n> +       enum component_type new_type;\n>         int err, lookup_flags = LOOKUP_NO_SYMLINKS;\n>\n>         if (ksmbd_override_fsids(work))\n> diff --git a/include/linux/namei.h b/include/linux/namei.h\n> index 58600cf234bc..fa3ae87762b7 100644\n> --- a/include/linux/namei.h\n> +++ b/include/linux/namei.h\n> @@ -16,7 +16,7 @@ enum { MAX_NESTED_LINKS = 8 };\n>  /*\n>   * Type of the last component on LOOKUP_PARENT\n>   */\n> -enum {LAST_NORM, LAST_ROOT, LAST_DOT, LAST_DOTDOT};\n> +enum component_type {LAST_NORM, LAST_ROOT, LAST_DOT, LAST_DOTDOT};\n\nPlease use last_type to match the LAST_ prefix.\n\nTBH, the thing that bothers me most is why is this enum exposed\noutside of namei.c\nin the first place?\n\n>\n>  /* pathwalk mode */\n>  #define LOOKUP_FOLLOW          BIT(0)  /* follow links at the end */\n> @@ -70,7 +70,7 @@ static inline void end_removing_path(const struct path *path , struct dentry *de\n>         end_creating_path(path, dentry);\n>  }\n>  int vfs_path_parent_lookup(struct filename *filename, unsigned int flags,\n> -                          struct path *parent, struct qstr *last, int *type,\n> +                          struct path *parent, struct qstr *last, enum component_type *type,\n>                            const struct path *root);\n\nAs your patch demonstrates, the only use of enum last_type outside of namei.c is\nin ksmbd calls to vfs_path_parent_lookup().\n\nThis is an \"internal\" lookup helper that was exposed by commit\n74d7970febf7e (\"ksmbd: fix racy issue from using ->d_parent and ->d_name\")\n\nThis commit appears to be using the helper inconsistently in\nits two call sites, because ksmbd_vfs_rename(), is not checking the\nunlikely(type != LAST_NORM) case.\n\nPerhaps it would be wiser not to expose enum last_type at all and move\nthe unlikely(type != LAST_NORM) check into the helper itself in namei.c.\n\nWDYT?\n\nThanks,\nAmir.","headers":{"Return-Path":"\n <linux-cifs+bounces-10935-incoming=patchwork.ozlabs.org@vger.kernel.org>","X-Original-To":["incoming@patchwork.ozlabs.org","linux-cifs@vger.kernel.org"],"Delivered-To":"patchwork-incoming@legolas.ozlabs.org","Authentication-Results":["legolas.ozlabs.org;\n\tdkim=pass (2048-bit key;\n unprotected) header.d=gmail.com header.i=@gmail.com header.a=rsa-sha256\n header.s=20251104 header.b=dW1b51ak;\n\tdkim-atps=neutral","legolas.ozlabs.org;\n spf=pass (sender SPF authorized) smtp.mailfrom=vger.kernel.org\n (client-ip=2600:3c0a:e001:db::12fc:5321; helo=sea.lore.kernel.org;\n envelope-from=linux-cifs+bounces-10935-incoming=patchwork.ozlabs.org@vger.kernel.org;\n receiver=patchwork.ozlabs.org)","smtp.subspace.kernel.org;\n\tdkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com\n header.b=\"dW1b51ak\"","smtp.subspace.kernel.org;\n arc=pass smtp.client-ip=209.85.218.47","smtp.subspace.kernel.org;\n dmarc=pass (p=none dis=none) header.from=gmail.com","smtp.subspace.kernel.org;\n spf=pass smtp.mailfrom=gmail.com"],"Received":["from sea.lore.kernel.org (sea.lore.kernel.org\n [IPv6:2600:3c0a:e001:db::12fc:5321])\n\t(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)\n\t key-exchange x25519 server-signature ECDSA (secp384r1) server-digest SHA384)\n\t(No client certificate requested)\n\tby legolas.ozlabs.org (Postfix) with ESMTPS id 4fzhZ1270lz1yGs\n\tfor <incoming@patchwork.ozlabs.org>; Mon, 20 Apr 2026 20:29:45 +1000 (AEST)","from smtp.subspace.kernel.org (conduit.subspace.kernel.org\n [100.90.174.1])\n\tby sea.lore.kernel.org (Postfix) with ESMTP id 4372D303A87E\n\tfor <incoming@patchwork.ozlabs.org>; Mon, 20 Apr 2026 10:29:30 +0000 (UTC)","from localhost.localdomain (localhost.localdomain [127.0.0.1])\n\tby smtp.subspace.kernel.org (Postfix) with ESMTP id E6CD837FF51;\n\tMon, 20 Apr 2026 10:29:29 +0000 (UTC)","from mail-ej1-f47.google.com (mail-ej1-f47.google.com\n [209.85.218.47])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits))\n\t(No client certificate requested)\n\tby smtp.subspace.kernel.org (Postfix) with ESMTPS id 3365A399341\n\tfor <linux-cifs@vger.kernel.org>; Mon, 20 Apr 2026 10:29:27 +0000 (UTC)","by mail-ej1-f47.google.com with SMTP id\n a640c23a62f3a-b9c6680aaf8so467753766b.3\n        for <linux-cifs@vger.kernel.org>;\n Mon, 20 Apr 2026 03:29:27 -0700 (PDT)"],"ARC-Seal":["i=2; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116;\n\tt=1776680969; cv=pass;\n b=GM2QUxNIZD7/wjLFCZuIkguRB9yx+lBnheuSFd6BrwW7doTFOgtTxMkBpSZMgHSewmLRq21aVQKZwfSZN6uxWS8AllcRDlke4EfBFf4DGHhLkGwDaNSH9SwYfCyvCLeRu40ZcATrgv077fQR6xMC/KV7CHeate543eCQN7eS0zA=","i=1; a=rsa-sha256; t=1776680965; cv=none;\n        d=google.com; s=arc-20240605;\n        b=ZKzodRbABFRksUBjROLyhpJZ7ZwEW4k3tU9EKtaVdtJ22ZlO+na5XOF5+q7dqPcVJq\n         BHkIDE5X5hD8zr/JQojxOyvVR/iffvBdDOXQRyxDegAFZj3doU6hpfQRJFOJJ/Epngvj\n         AghrnRg8FTyS4CYWmkNo0XtjU8athpDJLAIp+fhpWbBpAxgXQ4QGhmT5tw2UD2btGtOe\n         2Ez23nuZyGgC81Yw5ow3gSlbv02sW8IcmNTXjpio7K8HsYu65MY2zslFURjJjaXG58p+\n         +SA4koyHjwGKUqFsLpulNiw6pdP5Xjny9sWLadQr4Zu31r7oedffW2D1N+Z5VFYsOtHs\n         390Q=="],"ARC-Message-Signature":["i=2; a=rsa-sha256; d=subspace.kernel.org;\n\ts=arc-20240116; t=1776680969; c=relaxed/simple;\n\tbh=b1r7vji9w89PWRc9XU2AkOZofoPvtW87U/mgamZN7Z8=;\n\th=MIME-Version:References:In-Reply-To:From:Date:Message-ID:Subject:\n\t To:Cc:Content-Type;\n b=VtAzkHWEnag0HKukgo3DsOq1zDE6ypH9Yd6pS+/AE4JGS2PJ5ufr8lC3VLW0lSnfVQDEy4ASV2zdVlmKQj1Htxg4WR7p6hxgKpcZJQRatTWwv8jVgeN4YHqR6ZHbvcqdF5IBjd9eQcuA1WvtgHOU6woKHEaj/0XgqBotJXAsNUo=","i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com;\n s=arc-20240605;\n        h=content-transfer-encoding:cc:to:subject:message-id:date:from\n         :in-reply-to:references:mime-version:dkim-signature;\n        bh=w63l8zrtQNZhZJyil++3JdFvDsRqKJ7vTTWckv8Wa/g=;\n        fh=AKYtjnjvY5j80g1PqWub9kHsdVGG6sxRw7IBMQXWac8=;\n        b=FfehCC9DaxFnmAqP1RNPRveIbGY1Y4msw8B4kf1J9MhPfBaJ/tvl7uSwMrEwZwGrza\n         swzZIVNgJUsJf1F21tibWeAXRTyYHjBqeoOJO0fMSJJglSfSD4wKYDtBoP90u/rsu61W\n         IIJ5XoP9qGNnCKnokuytU/kUvsaRRKpu3qGbyD/fBi6l6dr9WnHjhKdgtrC5CImiaR0B\n         1lbNPsnS92QjLatbvcsmAGqiqU9uLzI0NO24428xnnHlf9Znq3ROPukxd9Gxc//4Wyua\n         bL9y5A78p4u1B6n69gVL0ToUMTaLi9x2osPYfWB2GucTWSgRCwgGq203hri+mFutfXc/\n         FEQw==;\n        darn=vger.kernel.org"],"ARC-Authentication-Results":["i=2; smtp.subspace.kernel.org;\n dmarc=pass (p=none dis=none) header.from=gmail.com;\n spf=pass smtp.mailfrom=gmail.com;\n dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com\n header.b=dW1b51ak; arc=pass smtp.client-ip=209.85.218.47","i=1; mx.google.com; arc=none"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n        d=gmail.com; s=20251104; t=1776680965; x=1777285765;\n darn=vger.kernel.org;\n        h=content-transfer-encoding:cc:to:subject:message-id:date:from\n         :in-reply-to:references:mime-version:from:to:cc:subject:date\n         :message-id:reply-to;\n        bh=w63l8zrtQNZhZJyil++3JdFvDsRqKJ7vTTWckv8Wa/g=;\n        b=dW1b51akAJ8lGCQr06LM3d7UsI1+mRdo5pQulQyadmKSAA6O3vhxDT+xrSCOUjvr7c\n         XCrdDB0U1gArrQkts6yjXG5D7Yz3VizSYQ6bzfGhzB1PofmblbThC/H50KvwxwmWaKE3\n         /kyrJc05izHiPsg93ak8oNuf9Wwnk33Jtkvwq/aqCtLX5hQqE8dZGIHOeO9lKlHh2XL6\n         EcYT4PTrJclQjZ1ojxtbNXHSWNPmx5gyJsyGValdFG0cpV+YE7sMJ99kUpFyFnTLzIUU\n         ncx3Xti8DAoRawd9FgM+vKyQEptPWYnJL5IWOZDgm9Iozu4rrbKIV1Ya7wqX280/tO6K\n         Cqww==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n        d=1e100.net; s=20251104; t=1776680965; x=1777285765;\n        h=content-transfer-encoding:cc:to:subject:message-id:date:from\n         :in-reply-to:references:mime-version:x-gm-gg:x-gm-message-state:from\n         :to:cc:subject:date:message-id:reply-to;\n        bh=w63l8zrtQNZhZJyil++3JdFvDsRqKJ7vTTWckv8Wa/g=;\n        b=kxDxlQe4lsCCcr6LeSm7j2hlY/Ifb6t1aALprPLml6kc/W13OBJ+uOivwa0otz0k2j\n         jLCCejGDMYbJm9BU/C5NEDnx/ad0RfWF2SHkl55NW0rEZ7y/+m0VZ+aTRgTRFGj3v9qx\n         gOJDJBwDuRTK+eSk4E8dmJ39ynzxI4Y38qjEpE3BgHSJyO+opvzfyvSb5DwxmuCvj7ca\n         NSkYj9MydZm/vRJFazR5L8EcdNbejfu3A42nqx2o8nM3aVUKwpb+rw7mRMkgrVNpqX0r\n         gq1zZykDlJ7GWbnCqUvnVYAvnAMrjo++eigCngIi0Vxjb4Kw29DISoMgZHu5DYF4itif\n         jhrA==","X-Forwarded-Encrypted":"i=1;\n AFNElJ/b2AAuxbW3dbKSJ/FQiZYZ/QoD6h155HXm783hjfCKPzBeNMcvGwOhM7w8XsBuOlV8m4hSwf/leosh@vger.kernel.org","X-Gm-Message-State":"AOJu0Yyh96hEEod8ElmJojdU5B40ejU/TzLQV9JEPY8N2zMoSnx9sbfx\n\tGwgpm/twcjSfXKSR+mlzJcZTgXBtz6CLeHZXq6EZm40yzghU8k1wUn5joGNYVvU/yh1Hyx14sPF\n\txV57Qt0ZDcUyUgTnxa2kY0X88lrxWUVU=","X-Gm-Gg":"AeBDieuHHRoIcw+8oR/3OVw27SV82G4awycttiG4HNalXvcCUHQvoYTwUwBlW1sz3P0\n\tHoovWfljdk4RHHEcHr97l/CzrxplTLrV2fmKRZksqbBM1h4um52TCiE56OfRfNn1nzL6C08yb+C\n\tCvqcqdB6olP74Rb+K66qeGdnut6KUpHzVY7t7pgHmw8QiDFiIKPRjhG85cU9f56Hf9ofScsyLo8\n\tfUnMuF3KxeOv0BYuCd7e0MRhMjy3Bt73XHBwYXDdAdJ3Xfi63DMtbggb1s//R+1JAgkGgZK4OJb\n\t9KkVkDDd1g2fKVtT/ahwPlsCSQTyWyC1pkiCBRDq+E8iBtx+jGyk","X-Received":"by 2002:a17:907:e84b:b0:ba3:8a04:7688 with SMTP id\n a640c23a62f3a-ba41b5d28acmr626277266b.36.1776680965266; Mon, 20 Apr 2026\n 03:29:25 -0700 (PDT)","Precedence":"bulk","X-Mailing-List":"linux-cifs@vger.kernel.org","List-Id":"<linux-cifs.vger.kernel.org>","List-Subscribe":"<mailto:linux-cifs+subscribe@vger.kernel.org>","List-Unsubscribe":"<mailto:linux-cifs+unsubscribe@vger.kernel.org>","MIME-Version":"1.0","References":"<20260419161620.564854-1-jkoolstra@xs4all.nl>","In-Reply-To":"<20260419161620.564854-1-jkoolstra@xs4all.nl>","From":"Amir Goldstein <amir73il@gmail.com>","Date":"Mon, 20 Apr 2026 12:29:12 +0200","X-Gm-Features":"AQROBzC40fUp9j6jgAwRhk99puzFCqWY_c5zsC-v2pmMmssjl54h3VngfWz6pUk","Message-ID":"\n <CAOQ4uxhh1kGnuY7sKTqti12tjFD4t1vCJ0qQrGngU2+0idXtpQ@mail.gmail.com>","Subject":"Re: [PATCH v2] vfs: replace ints with enum component_type for\n LAST_XXX","To":"Jori Koolstra <jkoolstra@xs4all.nl>","Cc":"Alexander Viro <viro@zeniv.linux.org.uk>,\n Christian Brauner <brauner@kernel.org>, Jan Kara <jack@suse.cz>,\n\tNamjae Jeon <linkinjeon@kernel.org>, Steve French <smfrench@gmail.com>,\n\tSergey Senozhatsky <senozhatsky@chromium.org>, Tom Talpey <tom@talpey.com>,\n NeilBrown <neil@brown.name>,\n\tJeff Layton <jlayton@kernel.org>, Mateusz Guzik <mjguzik@gmail.com>,\n\t\"open list:FILESYSTEMS (VFS and infrastructure)\"\n <linux-fsdevel@vger.kernel.org>, open list <linux-kernel@vger.kernel.org>,\n\t\"open list:KERNEL SMB3 SERVER (KSMBD)\" <linux-cifs@vger.kernel.org>","Content-Type":"text/plain; charset=\"UTF-8\"","Content-Transfer-Encoding":"quoted-printable"}},{"id":3680212,"web_url":"http://patchwork.ozlabs.org/comment/3680212/","msgid":"<20260422033952.GG3518998@ZenIV>","list_archive_url":null,"date":"2026-04-22T03:39:52","subject":"Re: [PATCH v2] vfs: replace ints with enum component_type for\n LAST_XXX","submitter":{"id":583,"url":"http://patchwork.ozlabs.org/api/people/583/","name":"Al Viro","email":"viro@ZenIV.linux.org.uk"},"content":"On Sun, Apr 19, 2026 at 06:16:16PM +0200, Jori Koolstra wrote:\n> Several functions in namei.c take an \"int *type\" parameter, such as\n> filename_parentat(). To know what values this can take you have to find\n> the anonymous struct that defines the LAST_XXX values.\n\nTo find _what_, again?\n\n> I would argue\n> that the readability of the code is improved by making this an explicit\n> type.\n\nDo argue, then.  How does that improve readability?\n\nWhat I see is a lot of churn.  Incidentally, I'm not at all sure that we\nshould expose those outside of fs/namei.c - if you look at the sole place\nwhere any of those are used anywhere else you'll see this:\n\terr = vfs_path_parent_lookup(filename, flags,\n\t\t\t\t     path, &last, &type,\n\t\t\t\t     root_share_path);\n\tif (err)\n\t\treturn err;\n\n\tif (unlikely(type != LAST_NORM)) {\n\t\tpath_put(path);\n\t\treturn -ENOENT;\n\t}\nwith the only other caller of vfs_path_parent_lookup() being\n\terr = vfs_path_parent_lookup(to, lookup_flags | LOOKUP_BENEATH,\n\t\t\t\t     &new_path, &new_last, &new_type,\n\t\t\t\t     &share_conf->vfs_path);\n\tif (err)\n\t\tgoto out1;\nand having no uses of new_type whatsoever.  Smells like missing\ncheck _and_ wrong calling conventions...\n\nDo we ever want to allow anything other thant LAST_NORM in any of\nthe users?","headers":{"Return-Path":"\n <linux-cifs+bounces-10996-incoming=patchwork.ozlabs.org@vger.kernel.org>","X-Original-To":["incoming@patchwork.ozlabs.org","linux-cifs@vger.kernel.org"],"Delivered-To":"patchwork-incoming@legolas.ozlabs.org","Authentication-Results":["legolas.ozlabs.org;\n\tdkim=pass (2048-bit key;\n unprotected) header.d=linux.org.uk header.i=@linux.org.uk header.a=rsa-sha256\n header.s=zeniv-20220401 header.b=wt8qWG5z;\n\tdkim-atps=neutral","legolas.ozlabs.org;\n spf=pass (sender SPF authorized) smtp.mailfrom=vger.kernel.org\n (client-ip=2600:3c04:e001:36c::12fc:5321; helo=tor.lore.kernel.org;\n envelope-from=linux-cifs+bounces-10996-incoming=patchwork.ozlabs.org@vger.kernel.org;\n receiver=patchwork.ozlabs.org)","smtp.subspace.kernel.org;\n\tdkim=pass (2048-bit key) header.d=linux.org.uk header.i=@linux.org.uk\n header.b=\"wt8qWG5z\"","smtp.subspace.kernel.org;\n arc=none smtp.client-ip=62.89.141.173","smtp.subspace.kernel.org;\n dmarc=pass (p=none dis=none) header.from=zeniv.linux.org.uk","smtp.subspace.kernel.org;\n spf=none smtp.mailfrom=ftp.linux.org.uk"],"Received":["from tor.lore.kernel.org (tor.lore.kernel.org\n [IPv6:2600:3c04:e001:36c::12fc:5321])\n\t(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)\n\t key-exchange x25519 server-signature ECDSA (secp384r1) server-digest SHA384)\n\t(No client certificate requested)\n\tby legolas.ozlabs.org (Postfix) with ESMTPS id 4g0lHg4g9mz1yD5\n\tfor <incoming@patchwork.ozlabs.org>; Wed, 22 Apr 2026 13:35:59 +1000 (AEST)","from smtp.subspace.kernel.org (conduit.subspace.kernel.org\n [100.90.174.1])\n\tby tor.lore.kernel.org (Postfix) with ESMTP id 9F67A30166D4\n\tfor <incoming@patchwork.ozlabs.org>; Wed, 22 Apr 2026 03:35:55 +0000 (UTC)","from localhost.localdomain (localhost.localdomain [127.0.0.1])\n\tby smtp.subspace.kernel.org (Postfix) with ESMTP id 95A3F39B95A;\n\tWed, 22 Apr 2026 03:35:52 +0000 (UTC)","from zeniv.linux.org.uk (zeniv.linux.org.uk [62.89.141.173])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))\n\t(No client certificate requested)\n\tby smtp.subspace.kernel.org (Postfix) with ESMTPS id ED3D02D2495;\n\tWed, 22 Apr 2026 03:35:49 +0000 (UTC)","from viro by zeniv.linux.org.uk with local (Exim 4.99.1 #2 (Red Hat\n Linux))\n\tid 1wFORM-00000005M8y-0rTv;\n\tWed, 22 Apr 2026 03:39:52 +0000"],"ARC-Seal":"i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116;\n\tt=1776828952; cv=none;\n b=EUuTBdDPKJStWf2z/8zP+JIFYO6Tb3vw7RYcDvNucEk3XmBNQTN19bs43ZP8rPnjD96/rBwgTJqJM4MH3LrGKDpkeCRs/t8GlWyy/Dz48csF8qqr6CfIWN4NITN2UgdAvGhvfQU2b10tZwl3hQbSRFceoJjIy9QmzZdx7tR+IOY=","ARC-Message-Signature":"i=1; a=rsa-sha256; d=subspace.kernel.org;\n\ts=arc-20240116; t=1776828952; c=relaxed/simple;\n\tbh=WY7RQRfgrHELx30NbuhdsbIsL2HRNeBipxfqBnreVyQ=;\n\th=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version:\n\t Content-Type:Content-Disposition:In-Reply-To;\n b=OX1tcaDc3ycJQ3BglxbRSak8I59iZYMa0RS2WUCCRbIfEgqZ3Gs0hKEqzE+Su7s/WL1NEYUezNFehrTLGg0ZGdewWaLhx2qTR04RivS4qB6xUFi437cgdYyBgLsMRZMZs5oLzx05TShxqp/vYOwaXZjPtM4RvqRGAa6z0soh2ek=","ARC-Authentication-Results":"i=1; smtp.subspace.kernel.org;\n dmarc=pass (p=none dis=none) header.from=zeniv.linux.org.uk;\n spf=none smtp.mailfrom=ftp.linux.org.uk;\n dkim=pass (2048-bit key) header.d=linux.org.uk header.i=@linux.org.uk\n header.b=wt8qWG5z; arc=none smtp.client-ip=62.89.141.173","DKIM-Signature":"v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed;\n\td=linux.org.uk; s=zeniv-20220401; h=Sender:In-Reply-To:Content-Type:\n\tMIME-Version:References:Message-ID:Subject:Cc:To:From:Date:Reply-To:\n\tContent-Transfer-Encoding:Content-ID:Content-Description;\n\tbh=bw2O6inGL+8Q/ZN4yIDCKfUACqQ2XYABbUQEv1jxunY=; b=wt8qWG5z1KwjZ/3ELqursGcjvU\n\tvxE51kOw0KIjR4Lu4gI5pxERUfILgjqodWbaG6w2AvOtdWpk6MDJ9xoEeXhYQp9CfylHeYykOzeLH\n\tlEWrYkGFX0TUe+ZcmwgWgL1uUpHa3PRwhiOMFfb1JG3YXKeL0y991BYADYXcvQn/7vyDGzjlMo+zP\n\te8HCXwRLNECWnaqT14NbtQAyetoGuZu6haspp/isVkH/ti+65T6VS8iXiCoqOiBSUlBKSHlGt9gzl\n\tagke43r+BbpHsafWarAB+GyiaOu8AsN75Gfn+9ZfBMHuSHVn7pEeS3H+Tnk3vQbOlmT/4Z5yycTUT\n\tD12NCzFQ==;","Date":"Wed, 22 Apr 2026 04:39:52 +0100","From":"Al Viro <viro@zeniv.linux.org.uk>","To":"Jori Koolstra <jkoolstra@xs4all.nl>","Cc":"Christian Brauner <brauner@kernel.org>, Jan Kara <jack@suse.cz>,\n\tNamjae Jeon <linkinjeon@kernel.org>,\n\tSteve French <smfrench@gmail.com>,\n\tSergey Senozhatsky <senozhatsky@chromium.org>,\n\tTom Talpey <tom@talpey.com>, NeilBrown <neil@brown.name>,\n\tAmir Goldstein <amir73il@gmail.com>,\n\tJeff Layton <jlayton@kernel.org>, Mateusz Guzik <mjguzik@gmail.com>,\n\t\"open list:FILESYSTEMS (VFS and infrastructure)\"\n <linux-fsdevel@vger.kernel.org>,\n\topen list <linux-kernel@vger.kernel.org>,\n\t\"open list:KERNEL SMB3 SERVER (KSMBD)\" <linux-cifs@vger.kernel.org>","Subject":"Re: [PATCH v2] vfs: replace ints with enum component_type for\n LAST_XXX","Message-ID":"<20260422033952.GG3518998@ZenIV>","References":"<20260419161620.564854-1-jkoolstra@xs4all.nl>","Precedence":"bulk","X-Mailing-List":"linux-cifs@vger.kernel.org","List-Id":"<linux-cifs.vger.kernel.org>","List-Subscribe":"<mailto:linux-cifs+subscribe@vger.kernel.org>","List-Unsubscribe":"<mailto:linux-cifs+unsubscribe@vger.kernel.org>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20260419161620.564854-1-jkoolstra@xs4all.nl>","Sender":"Al Viro <viro@ftp.linux.org.uk>"}},{"id":3680471,"web_url":"http://patchwork.ozlabs.org/comment/3680471/","msgid":"<2033530362.1019505.1776855149486@kpc.webmail.kpnmail.nl>","list_archive_url":null,"date":"2026-04-22T10:52:29","subject":"Re: [PATCH v2] vfs: replace ints with enum component_type for\n LAST_XXX","submitter":{"id":92813,"url":"http://patchwork.ozlabs.org/api/people/92813/","name":"Jori Koolstra","email":"jkoolstra@xs4all.nl"},"content":"Hi Al,\n\n> Op 22-04-2026 05:39 CEST schreef Al Viro <viro@zeniv.linux.org.uk>:\n> \n>  \n> On Sun, Apr 19, 2026 at 06:16:16PM +0200, Jori Koolstra wrote:\n> > Several functions in namei.c take an \"int *type\" parameter, such as\n> > filename_parentat(). To know what values this can take you have to find\n> > the anonymous struct that defines the LAST_XXX values.\n> \n> To find _what_, again?\n> \n> > I would argue\n> > that the readability of the code is improved by making this an explicit\n> > type.\n> \n> Do argue, then.  How does that improve readability?\n\nIt is certainly in part a matter of taste, so I certainly won't say this\nis \"wrong.\"\n\nThere is the matter of type safety, and the (possible) benefits this entails\nsuch as automatic case coverage checking for switch-statements. However, I don't\ndoubt for a second you aren't aware of this, so that probably won't convince\nyou :) . We indeed also don't do this for flag-types, but at least that\nis a kernel-wide established pattern (now I am curious how the Rust people\ndeal with flags).\n\nWhat I find somewhat unfortunate is that if I am looking at\n__filename_parentat(), for instance, the signature does not give me any\nclue what the purpose is of int *type. Then I look through the code and see\n\n\t\t*type = nd.last_type;\n\nand I have to jump to struct nameidata and it still tells me nothing. I have\nto grep last_type in nameidata to see it is used in conjunction with the \nanonymous enum.\n\nI can only tell that it would have slightly decreased cognitive load and jumping\naround for me when I first looked into namei.c. How that compares to some churn I\ncan't say.\n\n> \n> What I see is a lot of churn.  Incidentally, I'm not at all sure that we\n> should expose those outside of fs/namei.c - if you look at the sole place\n> where any of those are used anywhere else you'll see this:\n> \terr = vfs_path_parent_lookup(filename, flags,\n> \t\t\t\t     path, &last, &type,\n> \t\t\t\t     root_share_path);\n> \tif (err)\n> \t\treturn err;\n> \n> \tif (unlikely(type != LAST_NORM)) {\n> \t\tpath_put(path);\n> \t\treturn -ENOENT;\n> \t}\n> with the only other caller of vfs_path_parent_lookup() being\n> \terr = vfs_path_parent_lookup(to, lookup_flags | LOOKUP_BENEATH,\n> \t\t\t\t     &new_path, &new_last, &new_type,\n> \t\t\t\t     &share_conf->vfs_path);\n> \tif (err)\n> \t\tgoto out1;\n> and having no uses of new_type whatsoever.  Smells like missing\n> check _and_ wrong calling conventions...\n> \n> Do we ever want to allow anything other thant LAST_NORM in any of\n> the users?\n\nI agree with you and Amir. The naming is also a bit confusing. Maybe, I am\nconfusing myself but path_parentat() seems to return the parent component of the\npathname passed into nd, not the parent of the resolved path, and these do not\nhave to be the same if last_type isn't normal. I presume that is why they put\nthat check into vfs.c.\n\nSo what do you think is the best place to move \n\n \tif (unlikely(type != LAST_NORM)) {\n \t\tpath_put(path);\n \t\treturn -ENOENT;\n \t}\n\nto? path_parentat() or filename_parentat()?\n\nAnd thanks a lot for taking the time to review, Al, I appreciate it :)\n\nThanks,\nJori.","headers":{"Return-Path":"\n <linux-cifs+bounces-11006-incoming=patchwork.ozlabs.org@vger.kernel.org>","X-Original-To":["incoming@patchwork.ozlabs.org","linux-cifs@vger.kernel.org"],"Delivered-To":"patchwork-incoming@legolas.ozlabs.org","Authentication-Results":["legolas.ozlabs.org;\n\tdkim=pass (2048-bit key;\n secure) header.d=xs4all.nl header.i=@xs4all.nl header.a=rsa-sha256\n header.s=xs4all01 header.b=MB9QIKoB;\n\tdkim-atps=neutral","legolas.ozlabs.org;\n spf=pass (sender SPF authorized) smtp.mailfrom=vger.kernel.org\n (client-ip=172.232.135.74; helo=sto.lore.kernel.org;\n envelope-from=linux-cifs+bounces-11006-incoming=patchwork.ozlabs.org@vger.kernel.org;\n receiver=patchwork.ozlabs.org)","smtp.subspace.kernel.org;\n\tdkim=pass (2048-bit key) header.d=xs4all.nl header.i=@xs4all.nl\n header.b=\"MB9QIKoB\"","smtp.subspace.kernel.org;\n arc=none smtp.client-ip=195.121.94.183","smtp.subspace.kernel.org;\n dmarc=pass (p=reject dis=none) header.from=xs4all.nl","smtp.subspace.kernel.org;\n spf=pass smtp.mailfrom=xs4all.nl"],"Received":["from sto.lore.kernel.org (sto.lore.kernel.org [172.232.135.74])\n\t(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)\n\t key-exchange x25519 server-signature ECDSA (secp384r1) server-digest SHA384)\n\t(No client certificate requested)\n\tby legolas.ozlabs.org (Postfix) with ESMTPS id 4g0wzd3TZgz1yD5\n\tfor <incoming@patchwork.ozlabs.org>; Wed, 22 Apr 2026 20:52:45 +1000 (AEST)","from smtp.subspace.kernel.org (conduit.subspace.kernel.org\n [100.90.174.1])\n\tby sto.lore.kernel.org (Postfix) with ESMTP id 3A2CE300A315\n\tfor <incoming@patchwork.ozlabs.org>; Wed, 22 Apr 2026 10:52:42 +0000 (UTC)","from localhost.localdomain (localhost.localdomain [127.0.0.1])\n\tby smtp.subspace.kernel.org (Postfix) with ESMTP id B41593C13FD;\n\tWed, 22 Apr 2026 10:52:39 +0000 (UTC)","from ewsoutbound.kpnmail.nl (ewsoutbound.kpnmail.nl\n [195.121.94.183])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))\n\t(No client certificate requested)\n\tby smtp.subspace.kernel.org (Postfix) with ESMTPS id 32612351C3F\n\tfor <linux-cifs@vger.kernel.org>; Wed, 22 Apr 2026 10:52:37 +0000 (UTC)","from mta.kpnmail.nl (unknown [10.31.161.190])\n\tby ewsoutbound.so.kpn.org (Halon) with ESMTPS\n\tid 5b8e6a02-3e39-11f1-bea8-005056992ed3;\n\tWed, 22 Apr 2026 12:52:29 +0200 (CEST)","from mtaoutbound.kpnmail.nl (unknown [10.128.135.190])\n\tby mta.kpnmail.nl (Halon) with ESMTP\n\tid 5b9d35db-3e39-11f1-99cc-0050569977a2;\n\tWed, 22 Apr 2026 12:52:29 +0200 (CEST)","from cpxoxapps-mh08 (cpxoxapps-mh08.personalcloud.so.kpn.org\n [10.128.135.214])\n\tby mtaoutbound.kpnmail.nl (Halon) with ESMTPSA\n\tid 5b8a4992-3e39-11f1-b8d7-005056995d6c;\n\tWed, 22 Apr 2026 12:52:29 +0200 (CEST)"],"ARC-Seal":"i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116;\n\tt=1776855159; cv=none;\n b=Btua7hGa/LE/N1pKX4s9knNahhseDA05Lcee0UimIsaJyezK0Zu6UyDzHf8WNyIYfq8OG6QMmhIRJG1S72YXHUdTB1zThJCRpogSrUxBxvyG3UHrfKOY7hOvrWNYMfDnGOmoYw1r/6l/pRZ6FySZIjfNd4f9s+C46/DtB4ZXG88=","ARC-Message-Signature":"i=1; a=rsa-sha256; d=subspace.kernel.org;\n\ts=arc-20240116; t=1776855159; c=relaxed/simple;\n\tbh=n7ua3IBTaT1I2IjXE3Fgx7EVHWOjfMqpf703jfD3SMg=;\n\th=Date:From:To:Cc:Message-ID:In-Reply-To:References:Subject:\n\t MIME-Version:Content-Type;\n b=RrXV0Iq4W3uq19dTxdeudyaG9Rq0ELjyP44C6wMMp3mz3cU3nbVYNYFYvlqdBapJaETBYO39kbcgZIevAmK3V8+5YIOP9nKGsYitz3ogctxX5bX6F8OOoWxfToLE+IdaK46mPg9seWabt5nxwOaN2MJh/VNGmP9sgEddzv4avCo=","ARC-Authentication-Results":"i=1; smtp.subspace.kernel.org;\n dmarc=pass (p=reject dis=none) header.from=xs4all.nl;\n spf=pass smtp.mailfrom=xs4all.nl;\n dkim=pass (2048-bit key) header.d=xs4all.nl header.i=@xs4all.nl\n header.b=MB9QIKoB; arc=none smtp.client-ip=195.121.94.183","X-KPN-MessageId":"5b8e6a02-3e39-11f1-bea8-005056992ed3","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=xs4all.nl; s=xs4all01;\n\th=content-type:mime-version:subject:message-id:to:from:date;\n\tbh=lzobZnGX3W1YHn5mkFfzRdme5gItKXvBwoKmmwvYuYE=;\n\tb=MB9QIKoBhYGJTaPRIxuMVE0UQUOVn5hpU6PZBDVnEv2N5ka7Y1xx6qPJaJAnxNpPe+PGePupkcXR3\n\t keASuo99lRePl5/IrRo4fmKHnPT47pAVUYyKfeMGw0NYO34ZRCPMrrr4NGF2LxTHHdRAUw5/kWHXyK\n\t la3JVKrtaQH1i44wS8WxRp1eDQ365tGsE5tINDXh6K3+A6arr5J91d4FGhNVtFZXMseNA2s4xZZJwL\n\t U54RFI928yCmQNyzxXGVhLb9jlBuYV82pxHAWY7g4u9zTXLYw1EiwMOaSVtuUpYB1yy9RJDn0pN6Im\n\t xpGu2ji8MLf619bNY835SNHnEdWT/iA==","X-KPN-MID":"33|xYmwcf1RlbXBSVW5pKHQRa4Le0Yr1IznKtlN52MTrEXCcQF0ejXqNFYiUc0I3DJ\n OYd+48D9GfLHX4T7LIg8gnxMMfvhtP0ZS/iNIE46CnFw=","X-CMASSUN":"33|Q91q0BWVgfkvoiGVF0kbRuMY3WwmrpvGX1u+s0vBkvPjeNI6td+AD8y2uyZF77I\n tVP8iuO9otNNc40cuf0LjdA==","X-KPN-VerifiedSender":"Yes","Date":"Wed, 22 Apr 2026 12:52:29 +0200 (CEST)","From":"Jori Koolstra <jkoolstra@xs4all.nl>","To":"Al Viro <viro@zeniv.linux.org.uk>","Cc":"Christian Brauner <brauner@kernel.org>, Jan Kara <jack@suse.cz>,\n\tNamjae Jeon <linkinjeon@kernel.org>,\n\tSteve French <smfrench@gmail.com>,\n\tSergey Senozhatsky <senozhatsky@chromium.org>,\n\tTom Talpey <tom@talpey.com>, NeilBrown <neil@brown.name>,\n\tAmir Goldstein <amir73il@gmail.com>,\n\tJeff Layton <jlayton@kernel.org>, Mateusz Guzik <mjguzik@gmail.com>,\n\t\"open list:FILESYSTEMS (VFS and infrastructure)\"\n <linux-fsdevel@vger.kernel.org>, open list <linux-kernel@vger.kernel.org>,\n\t\"open list:KERNEL SMB3 SERVER (KSMBD)\" <linux-cifs@vger.kernel.org>","Message-ID":"<2033530362.1019505.1776855149486@kpc.webmail.kpnmail.nl>","In-Reply-To":"<20260422033952.GG3518998@ZenIV>","References":"<20260419161620.564854-1-jkoolstra@xs4all.nl>\n <20260422033952.GG3518998@ZenIV>","Subject":"Re: [PATCH v2] vfs: replace ints with enum component_type for\n LAST_XXX","Precedence":"bulk","X-Mailing-List":"linux-cifs@vger.kernel.org","List-Id":"<linux-cifs.vger.kernel.org>","List-Subscribe":"<mailto:linux-cifs+subscribe@vger.kernel.org>","List-Unsubscribe":"<mailto:linux-cifs+unsubscribe@vger.kernel.org>","MIME-Version":"1.0","Content-Type":"text/plain; charset=UTF-8","Content-Transfer-Encoding":"7bit","X-Priority":"3","Importance":"Normal"}},{"id":3680489,"web_url":"http://patchwork.ozlabs.org/comment/3680489/","msgid":"<358172838.1020532.1776855550028@kpc.webmail.kpnmail.nl>","list_archive_url":null,"date":"2026-04-22T10:59:10","subject":"Re: [PATCH v2] vfs: replace ints with enum component_type for\n LAST_XXX","submitter":{"id":92813,"url":"http://patchwork.ozlabs.org/api/people/92813/","name":"Jori Koolstra","email":"jkoolstra@xs4all.nl"},"content":"> Op 20-04-2026 12:29 CEST schreef Amir Goldstein <amir73il@gmail.com>:\n> \n> Please update documentation entry for last_type in\n> Documentation/filesystems/path-lookup.rst\n> \n\nYes, thanks for the reminder!\n\n> > -       unsigned        depth;\n> > -       int             total_link_count;\n> > +       struct path             path;\n> > +       struct qstr             last;\n> > +       struct path             root;\n> > +       struct inode            *inode; /* path.dentry.d_inode */\n> > +       unsigned int            flags, state;\n> > +       unsigned                seq, next_seq, m_seq, r_seq;\n> > +       enum component_type     last_type;\n> \n> I really dislike all this white space churn just because the enum name\n> is so long. If it was long for a good reason - increasing readability -\n> I may have conceded, but I don't think that is the case.\n> \n> I do not think that 'component_type' is more clear than 'last_type'\n> for a developer passing by reading the code.\n> If anything, the opposite, because 'last_type' or 'type' variables always\n> appear in the same function with 'last' variable, so it is more clear WHICH\n> object the type is referring to and matches the LAST_ enum prefix.\n\nOK, I see your point. I have changed it back. The reason I preferred\ncomponent_type is that the whole \"last\" thing is just incidental. There is\nnothing dependent on these components being \"last\", it's just that the\nenum values are only used in that context. It would make more sense to\nrestrict the enum to namei.c and use NORMAL, DOT, or something like that.\nBut I do agree that isn't worth the churn :) \n\n> \n> > +       unsigned                depth;\n> > +       int                     total_link_count;\n> >         struct saved {\n> >                 struct path link;\n> >                 struct delayed_call done;\n> > @@ -2221,7 +2221,7 @@ static struct dentry *follow_dotdot(struct nameidata *nd)\n> >         return dget(nd->path.dentry);\n> >  }\n> >\n> > -static const char *handle_dots(struct nameidata *nd, int type)\n> > +static const char *handle_dots(struct nameidata *nd, enum component_type type)\n> >  {\n> >         if (type == LAST_DOTDOT) {\n> >                 const char *error = NULL;\n> > @@ -2869,7 +2869,7 @@ static int path_parentat(struct nameidata *nd, unsigned flags,\n> >  /* Note: this does not consume \"name\" */\n> >  static int __filename_parentat(int dfd, struct filename *name,\n> >                                unsigned int flags, struct path *parent,\n> > -                              struct qstr *last, int *type,\n> > +                              struct qstr *last, enum component_type *type,\n> >                                const struct path *root)\n> >  {\n> >         int retval;\n> > @@ -2894,7 +2894,7 @@ static int __filename_parentat(int dfd, struct filename *name,\n> >\n> >  static int filename_parentat(int dfd, struct filename *name,\n> >                              unsigned int flags, struct path *parent,\n> > -                            struct qstr *last, int *type)\n> > +                            struct qstr *last, enum component_type *type)\n> >  {\n> >         return __filename_parentat(dfd, name, flags, parent, last, type, NULL);\n> >  }\n> > @@ -2963,7 +2963,8 @@ static struct dentry *__start_removing_path(int dfd, struct filename *name,\n> >         struct path parent_path __free(path_put) = {};\n> >         struct dentry *d;\n> >         struct qstr last;\n> > -       int type, error;\n> > +       enum component_type type;\n> > +       int error;\n> >\n> >         error = filename_parentat(dfd, name, 0, &parent_path, &last, &type);\n> >         if (error)\n> > @@ -3009,7 +3010,8 @@ struct dentry *kern_path_parent(const char *name, struct path *path)\n> >         CLASS(filename_kernel, filename)(name);\n> >         struct dentry *d;\n> >         struct qstr last;\n> > -       int type, error;\n> > +       enum component_type type;\n> > +       int error;\n> >\n> >         error = filename_parentat(AT_FDCWD, filename, 0, &parent_path, &last, &type);\n> >         if (error)\n> > @@ -3057,7 +3059,7 @@ EXPORT_SYMBOL(kern_path);\n> >   * @root: pointer to struct path of the base directory\n> >   */\n> >  int vfs_path_parent_lookup(struct filename *filename, unsigned int flags,\n> > -                          struct path *parent, struct qstr *last, int *type,\n> > +                          struct path *parent, struct qstr *last, enum component_type *type,\n> >                            const struct path *root)\n> >  {\n> >         return  __filename_parentat(AT_FDCWD, filename, flags, parent, last,\n> > @@ -4903,7 +4905,7 @@ static struct dentry *filename_create(int dfd, struct filename *name,\n> >         bool want_dir = lookup_flags & LOOKUP_DIRECTORY;\n> >         unsigned int reval_flag = lookup_flags & LOOKUP_REVAL;\n> >         unsigned int create_flags = LOOKUP_CREATE | LOOKUP_EXCL;\n> > -       int type;\n> > +       enum component_type type;\n> >         int error;\n> >\n> >         error = filename_parentat(dfd, name, reval_flag, path, &last, &type);\n> > @@ -5365,7 +5367,7 @@ int filename_rmdir(int dfd, struct filename *name)\n> >         struct dentry *dentry;\n> >         struct path path;\n> >         struct qstr last;\n> > -       int type;\n> > +       enum component_type type;\n> >         unsigned int lookup_flags = 0;\n> >         struct delegated_inode delegated_inode = { };\n> >  retry:\n> > @@ -5383,6 +5385,7 @@ int filename_rmdir(int dfd, struct filename *name)\n> >         case LAST_ROOT:\n> >                 error = -EBUSY;\n> >                 goto exit2;\n> > +       case LAST_NORM: ; // OK\n> \n> I was not aware that the compiler warns about missing cases for enums -\n> cool feature.\n> but that needs to be a break; rather than fallthough the end\n> and as a matter of taste, I would put the valid case first - not critical.\n> \n> >         }\n> >\n> >         error = mnt_want_write(path.mnt);\n> > @@ -5507,7 +5510,7 @@ int filename_unlinkat(int dfd, struct filename *name)\n> >         struct dentry *dentry;\n> >         struct path path;\n> >         struct qstr last;\n> > -       int type;\n> > +       enum component_type type;\n> >         struct inode *inode;\n> >         struct delegated_inode delegated_inode = { };\n> >         unsigned int lookup_flags = 0;\n> > @@ -6074,7 +6077,7 @@ int filename_renameat2(int olddfd, struct filename *from,\n> >         struct renamedata rd;\n> >         struct path old_path, new_path;\n> >         struct qstr old_last, new_last;\n> > -       int old_type, new_type;\n> > +       enum component_type old_type, new_type;\n> >         struct delegated_inode delegated_inode = { };\n> >         unsigned int lookup_flags = 0;\n> >         bool should_retry = false;\n> > diff --git a/fs/smb/server/vfs.c b/fs/smb/server/vfs.c\n> > index d08973b288e5..b12d481f5ba9 100644\n> > --- a/fs/smb/server/vfs.c\n> > +++ b/fs/smb/server/vfs.c\n> > @@ -56,7 +56,8 @@ static int ksmbd_vfs_path_lookup(struct ksmbd_share_config *share_conf,\n> >  {\n> >         struct qstr last;\n> >         const struct path *root_share_path = &share_conf->vfs_path;\n> > -       int err, type;\n> > +       int err;\n> > +       enum component_type type;\n> >         struct dentry *d;\n> >\n> >         if (pathname[0] == '\\0') {\n> > @@ -668,7 +669,7 @@ int ksmbd_vfs_rename(struct ksmbd_work *work, const struct path *old_path,\n> >         struct renamedata rd;\n> >         struct ksmbd_share_config *share_conf = work->tcon->share_conf;\n> >         struct ksmbd_file *parent_fp;\n> > -       int new_type;\n> > +       enum component_type new_type;\n> >         int err, lookup_flags = LOOKUP_NO_SYMLINKS;\n> >\n> >         if (ksmbd_override_fsids(work))\n> > diff --git a/include/linux/namei.h b/include/linux/namei.h\n> > index 58600cf234bc..fa3ae87762b7 100644\n> > --- a/include/linux/namei.h\n> > +++ b/include/linux/namei.h\n> > @@ -16,7 +16,7 @@ enum { MAX_NESTED_LINKS = 8 };\n> >  /*\n> >   * Type of the last component on LOOKUP_PARENT\n> >   */\n> > -enum {LAST_NORM, LAST_ROOT, LAST_DOT, LAST_DOTDOT};\n> > +enum component_type {LAST_NORM, LAST_ROOT, LAST_DOT, LAST_DOTDOT};\n> \n> Please use last_type to match the LAST_ prefix.\n> \n> TBH, the thing that bothers me most is why is this enum exposed\n> outside of namei.c\n> in the first place?\n> \n> >\n> >  /* pathwalk mode */\n> >  #define LOOKUP_FOLLOW          BIT(0)  /* follow links at the end */\n> > @@ -70,7 +70,7 @@ static inline void end_removing_path(const struct path *path , struct dentry *de\n> >         end_creating_path(path, dentry);\n> >  }\n> >  int vfs_path_parent_lookup(struct filename *filename, unsigned int flags,\n> > -                          struct path *parent, struct qstr *last, int *type,\n> > +                          struct path *parent, struct qstr *last, enum component_type *type,\n> >                            const struct path *root);\n> \n> As your patch demonstrates, the only use of enum last_type outside of namei.c is\n> in ksmbd calls to vfs_path_parent_lookup().\n> \n> This is an \"internal\" lookup helper that was exposed by commit\n> 74d7970febf7e (\"ksmbd: fix racy issue from using ->d_parent and ->d_name\")\n> \n> This commit appears to be using the helper inconsistently in\n> its two call sites, because ksmbd_vfs_rename(), is not checking the\n> unlikely(type != LAST_NORM) case.\n> \n> Perhaps it would be wiser not to expose enum last_type at all and move\n> the unlikely(type != LAST_NORM) check into the helper itself in namei.c.\n> \n> WDYT?\n> \n\nAgreed. See my email to Al in this thread.\n\n> Thanks,\n> Amir.\n\nThanks,\nJori.","headers":{"Return-Path":"\n <linux-cifs+bounces-11007-incoming=patchwork.ozlabs.org@vger.kernel.org>","X-Original-To":["incoming@patchwork.ozlabs.org","linux-cifs@vger.kernel.org"],"Delivered-To":"patchwork-incoming@legolas.ozlabs.org","Authentication-Results":["legolas.ozlabs.org;\n\tdkim=pass (2048-bit key;\n secure) header.d=xs4all.nl header.i=@xs4all.nl header.a=rsa-sha256\n header.s=xs4all01 header.b=iTPI6XYP;\n\tdkim-atps=neutral","legolas.ozlabs.org;\n spf=pass (sender SPF authorized) smtp.mailfrom=vger.kernel.org\n (client-ip=2600:3c0a:e001:db::12fc:5321; helo=sea.lore.kernel.org;\n envelope-from=linux-cifs+bounces-11007-incoming=patchwork.ozlabs.org@vger.kernel.org;\n receiver=patchwork.ozlabs.org)","smtp.subspace.kernel.org;\n\tdkim=pass (2048-bit key) header.d=xs4all.nl header.i=@xs4all.nl\n header.b=\"iTPI6XYP\"","smtp.subspace.kernel.org;\n arc=none smtp.client-ip=195.121.94.184","smtp.subspace.kernel.org;\n dmarc=pass (p=reject dis=none) header.from=xs4all.nl","smtp.subspace.kernel.org;\n spf=pass smtp.mailfrom=xs4all.nl"],"Received":["from sea.lore.kernel.org (sea.lore.kernel.org\n [IPv6:2600:3c0a:e001:db::12fc:5321])\n\t(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)\n\t key-exchange x25519)\n\t(No client certificate requested)\n\tby legolas.ozlabs.org (Postfix) with ESMTPS id 4g0x8Z68Lbz1y2d\n\tfor <incoming@patchwork.ozlabs.org>; Wed, 22 Apr 2026 21:00:30 +1000 (AEST)","from smtp.subspace.kernel.org (conduit.subspace.kernel.org\n [100.90.174.1])\n\tby sea.lore.kernel.org (Postfix) with ESMTP id F131F30221C1\n\tfor <incoming@patchwork.ozlabs.org>; Wed, 22 Apr 2026 10:59:21 +0000 (UTC)","from localhost.localdomain (localhost.localdomain [127.0.0.1])\n\tby smtp.subspace.kernel.org (Postfix) with ESMTP id CB8B63CE49F;\n\tWed, 22 Apr 2026 10:59:20 +0000 (UTC)","from ewsoutbound.kpnmail.nl (ewsoutbound.kpnmail.nl\n [195.121.94.184])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))\n\t(No client certificate requested)\n\tby smtp.subspace.kernel.org (Postfix) with ESMTPS id 0FE273C13E2\n\tfor <linux-cifs@vger.kernel.org>; Wed, 22 Apr 2026 10:59:17 +0000 (UTC)","from mta.kpnmail.nl (unknown [10.31.161.188])\n\tby ewsoutbound.so.kpn.org (Halon) with ESMTPS\n\tid 4a4b0fca-3e3a-11f1-afdd-005056994fde;\n\tWed, 22 Apr 2026 12:59:10 +0200 (CEST)","from mtaoutbound.kpnmail.nl (unknown [10.128.135.189])\n\tby mta.kpnmail.nl (Halon) with ESMTP\n\tid 4a5521df-3e3a-11f1-80f3-00505699693e;\n\tWed, 22 Apr 2026 12:59:10 +0200 (CEST)","from cpxoxapps-mh08 (cpxoxapps-mh08.personalcloud.so.kpn.org\n [10.128.135.214])\n\tby mtaoutbound.kpnmail.nl (Halon) with ESMTPSA\n\tid 4a4622fa-3e3a-11f1-94b1-00505699eff2;\n\tWed, 22 Apr 2026 12:59:10 +0200 (CEST)"],"ARC-Seal":"i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116;\n\tt=1776855560; cv=none;\n b=Ez+ym4lAukHCuJUNLbcjUNwNtpMu7bHy+khH9q9kothb0kLnEu62LKTjbByv/bGTd8ES1AZbOCluDQMZjcRtD+pu70J1GOiYD6GBcpQKSoVs8rUPid2TqxmINJ6Vo65WlIQuaO4LvwUYf868XjrJX5dTZurFQqbgsG6zBjf8a3M=","ARC-Message-Signature":"i=1; a=rsa-sha256; d=subspace.kernel.org;\n\ts=arc-20240116; t=1776855560; c=relaxed/simple;\n\tbh=fmI1LE/7ouZ+T6zgVX97t848L2T3YbV0wMRnT3P6XTk=;\n\th=Date:From:To:Cc:Message-ID:In-Reply-To:References:Subject:\n\t MIME-Version:Content-Type;\n b=gTjh94EEVc54DAAUfPcOmYWS5FChHZGoj4I/7+Zuse8g04sveH5M7vJFJNziQ1AvOcgpRHcYYmtWPwQK9NyBtxHNvfcjdPlGZTR9P5ZyxHe5JVxzWRBJEglltvA/WsH9BC6M1YHq5DW8I/e6slEX8c96Vv5STQ7HwD5p0wisKVc=","ARC-Authentication-Results":"i=1; smtp.subspace.kernel.org;\n dmarc=pass (p=reject dis=none) header.from=xs4all.nl;\n spf=pass smtp.mailfrom=xs4all.nl;\n dkim=pass (2048-bit key) header.d=xs4all.nl header.i=@xs4all.nl\n header.b=iTPI6XYP; arc=none smtp.client-ip=195.121.94.184","X-KPN-MessageId":"4a4b0fca-3e3a-11f1-afdd-005056994fde","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=xs4all.nl; s=xs4all01;\n\th=content-type:mime-version:subject:message-id:to:from:date;\n\tbh=vcllORq6duUusvXfjvMJ+rNXZ13ip/Bca/9oQnX5iVw=;\n\tb=iTPI6XYPOZmLV5JJupGXLtiPrb133R+RI24MirLKoLOUhLfDF6ZMU6Ie9rhCYID38D10yIYztZA+p\n\t gO3IwcvVQgxIQBClHvGVQRXNaN38eVfURd/NS8ND62MfN1AzkhAZEvi+iF9hop+JvB2MmUdO1P1EfW\n\t APX/KdROQHOrUAGgplQ2zWyOzjtpHUCfl7YVVsoo/ugw653XC6MR4dGw+N6ztqO1aJC9jxoGf8bBI3\n\t QW4SUW0SWnUSL21KTZpQPeYUs8m2Oqn0i9KIdSgcF9jl2zcb3/dDC6vdFtrihsmGrpTzypQw1R7Vy5\n\t IhrHxkWxVDUjYW9d+aR5a51C3xdPcNA==","X-KPN-MID":"33|LapmFKO7ER9iNGgpfHKbXxPiTPk1aJEXdRlxxN05ixQG0u0q2cLgq6e63D5lXLz\n YvAUdzJEaYFXttdVemuEeQ1D2cIK844eLVgxmHejWdFI=","X-CMASSUN":"33|dPpvh8M7Ocb7f7BhCWXZZq52eZmto7GGFzsqb3fDAnadmyCOJKA77sCJaQ1I2A8\n YsdyRUie3dv0WX+aJTlX9eQ==","X-KPN-VerifiedSender":"Yes","Date":"Wed, 22 Apr 2026 12:59:10 +0200 (CEST)","From":"Jori Koolstra <jkoolstra@xs4all.nl>","To":"Amir Goldstein <amir73il@gmail.com>","Cc":"Alexander Viro <viro@zeniv.linux.org.uk>,\n\tChristian Brauner <brauner@kernel.org>, Jan Kara <jack@suse.cz>,\n\tNamjae Jeon <linkinjeon@kernel.org>,\n\tSteve French <smfrench@gmail.com>,\n\tSergey Senozhatsky <senozhatsky@chromium.org>,\n\tTom Talpey <tom@talpey.com>, NeilBrown <neil@brown.name>,\n\tJeff Layton <jlayton@kernel.org>, Mateusz Guzik <mjguzik@gmail.com>,\n\t\"open list:FILESYSTEMS (VFS and infrastructure)\"\n <linux-fsdevel@vger.kernel.org>, open list <linux-kernel@vger.kernel.org>,\n\t\"open list:KERNEL SMB3 SERVER (KSMBD)\" <linux-cifs@vger.kernel.org>","Message-ID":"<358172838.1020532.1776855550028@kpc.webmail.kpnmail.nl>","In-Reply-To":"\n <CAOQ4uxhh1kGnuY7sKTqti12tjFD4t1vCJ0qQrGngU2+0idXtpQ@mail.gmail.com>","References":"<20260419161620.564854-1-jkoolstra@xs4all.nl>\n <CAOQ4uxhh1kGnuY7sKTqti12tjFD4t1vCJ0qQrGngU2+0idXtpQ@mail.gmail.com>","Subject":"Re: [PATCH v2] vfs: replace ints with enum component_type for\n LAST_XXX","Precedence":"bulk","X-Mailing-List":"linux-cifs@vger.kernel.org","List-Id":"<linux-cifs.vger.kernel.org>","List-Subscribe":"<mailto:linux-cifs+subscribe@vger.kernel.org>","List-Unsubscribe":"<mailto:linux-cifs+unsubscribe@vger.kernel.org>","MIME-Version":"1.0","Content-Type":"text/plain; charset=UTF-8","Content-Transfer-Encoding":"7bit","X-Priority":"3","Importance":"Normal"}}]