Received-SPF: pass (google.com: domain of linux-kernel+bounces-214000-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Date: Thu, 13 Jun 2024 21:40:47 +0100
From: Al Viro <viro@zeniv.linux.org.uk>
To: Abhinav Jain <jain.abhinav177@gmail.com>
Cc: dushistov@mail.ru, linux-kernel@vger.kernel.org,
	skhan@linuxfoundation.org, javier.carrasco.cruz@gmail.com
Subject: Re: [PATCH] ufs: Add table lookup to set d_type based on mode &
 S_IFMT
Message-ID: <20240613204047.GJ1629371@ZenIV>
References: <20240613202650.39739-1-jain.abhinav177@gmail.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20240613202650.39739-1-jain.abhinav177@gmail.com>
Sender: Al Viro <viro@ftp.linux.org.uk>

On Thu, Jun 13, 2024 at 08:26:50PM +0000, Abhinav Jain wrote:
> Add usf_de_type_mapping structure to map file mode masks to dir entries.
> Add a static ufs_type_table array to map file mode to dir entries.
> Remove switch and add a table based lookup for the mapping.
> Add ARRAY_SIZE macro on ufs_type_table to eliminate checkpatch warning.

For one thing, ARRAY_SIZE is already defined.  Finding the header
in question (and figuring out if its include needs to be added) is
left as an exercise for reader.

For another, you have added a copy of that array to *every* *file*
that inludes "util.h".  Finding the number of those files is, again,
left as an exercise for reader.

Finally, this

> +	for (i = 0; i < ARRAY_SIZE(ufs_type_table); i++) {
> +		if ((mode & S_IFMT) == ufs_type_table[i].mode_mask) {
> +			de->d_u.d_44.d_type = ufs_type_table[i].d_type;
> +			break;
> +		}

should've raised an arseload of mental red flags.  That loop is
bloody ridiculous, even if you don't bother to check what other
similar filesystems actually do in the counterpart of that logics.

"Table lookup" does *NOT* refer to that.  What you've got is strictly
worse than the original switch, and that takes some doing.

NAK.