On Wed, Mar 09, 2022 at 11:22:47AM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@xxxxxxxxxx>
>
> The symbol xfs_name_dotdot is a global variable that the xfs codebase
> uses here and there to look up directory dotdot entries. Currently it's
> a non-const variable, which means that it's a mutable global variable.
> So far nobody's abused this to cause problems, but let's use the
> compiler to enforce that.
>
> Signed-off-by: Darrick J. Wong <djwong@xxxxxxxxxx>
> ---
> fs/xfs/libxfs/xfs_dir2.c | 6 +++++-
> fs/xfs/libxfs/xfs_dir2.h | 2 +-
> fs/xfs/scrub/parent.c | 6 ++++--
> fs/xfs/xfs_export.c | 3 ++-
> 4 files changed, 12 insertions(+), 5 deletions(-)
>
>
> diff --git a/fs/xfs/libxfs/xfs_dir2.c b/fs/xfs/libxfs/xfs_dir2.c
> index a77d931d65a3..cf9fa07e62d5 100644
> --- a/fs/xfs/libxfs/xfs_dir2.c
> +++ b/fs/xfs/libxfs/xfs_dir2.c
> @@ -19,7 +19,11 @@
> #include "xfs_error.h"
> #include "xfs_trace.h"
>
> -struct xfs_name xfs_name_dotdot = { (unsigned char *)"..", 2, XFS_DIR3_FT_DIR };
> +const struct xfs_name xfs_name_dotdot = {
> + .name = (unsigned char *)"..",
> + .len = 2,
> + .type = XFS_DIR3_FT_DIR,
> +};
*nod*
> diff --git a/fs/xfs/scrub/parent.c b/fs/xfs/scrub/parent.c
> index ab182a5cd0c0..e9549d998cdc 100644
> --- a/fs/xfs/scrub/parent.c
> +++ b/fs/xfs/scrub/parent.c
> @@ -131,6 +131,7 @@ xchk_parent_validate(
> xfs_ino_t dnum,
> bool *try_again)
> {
> + struct xfs_name dotdot = xfs_name_dotdot;
> struct xfs_mount *mp = sc->mp;
> struct xfs_inode *dp = NULL;
> xfs_nlink_t expected_nlink;
> @@ -230,7 +231,7 @@ xchk_parent_validate(
> expected_nlink = VFS_I(sc->ip)->i_nlink == 0 ? 0 : 1;
>
> /* Look up '..' to see if the inode changed. */
> - error = xfs_dir_lookup(sc->tp, sc->ip, &xfs_name_dotdot, &dnum, NULL);
> + error = xfs_dir_lookup(sc->tp, sc->ip, &dotdot, &dnum, NULL);
> if (!xchk_fblock_process_error(sc, XFS_DATA_FORK, 0, &error))
> goto out_rele;
>
Why can't xfs_dir_lookup() be defined as a const xname for the input
name? All it does is extract the contents into the da_args fields,
and pass it to xfs_dir_hashname() which you converted to const in
the previous patch.
Or does the compiler then complain at all the other callsites that
you're passing non-const stuff to const function parameters? i.e. am
I just pulling on another dangling end of the ball of string at this
point?
> @@ -263,6 +264,7 @@ int
> xchk_parent(
> struct xfs_scrub *sc)
> {
> + struct xfs_name dotdot = xfs_name_dotdot;
> struct xfs_mount *mp = sc->mp;
> xfs_ino_t dnum;
> bool try_again;
> @@ -293,7 +295,7 @@ xchk_parent(
> xfs_iunlock(sc->ip, XFS_ILOCK_EXCL | XFS_MMAPLOCK_EXCL);
>
> /* Look up '..' */
> - error = xfs_dir_lookup(sc->tp, sc->ip, &xfs_name_dotdot, &dnum, NULL);
> + error = xfs_dir_lookup(sc->tp, sc->ip, &dotdot, &dnum, NULL);
> if (!xchk_fblock_process_error(sc, XFS_DATA_FORK, 0, &error))
> goto out;
> if (!xfs_verify_dir_ino(mp, dnum)) {
> diff --git a/fs/xfs/xfs_export.c b/fs/xfs/xfs_export.c
> index 1064c2342876..8939119191f4 100644
> --- a/fs/xfs/xfs_export.c
> +++ b/fs/xfs/xfs_export.c
> @@ -206,10 +206,11 @@ STATIC struct dentry *
> xfs_fs_get_parent(
> struct dentry *child)
> {
> + struct xfs_name dotdot = xfs_name_dotdot;
> int error;
> struct xfs_inode *cip;
>
> - error = xfs_lookup(XFS_I(d_inode(child)), &xfs_name_dotdot, &cip, NULL);
> + error = xfs_lookup(XFS_I(d_inode(child)), &dotdot, &cip, NULL);
> if (unlikely(error))
> return ERR_PTR(error);
This only calls xfs_dir_lookup() with name, so if xfs_dir_lookup()
can have a const name, so can xfs_lookup()....
Cheers,
Dave.
--
Dave Chinner
david@xxxxxxxxxxxxx
