xfs
[Top] [All Lists]

Re: [PATCH 2/2] xfs_repair: Properly set lost+found inode as used

To: Carlos Maiolino <cmaiolino@xxxxxxxxxx>
Subject: Re: [PATCH 2/2] xfs_repair: Properly set lost+found inode as used
From: Christoph Hellwig <hch@xxxxxxxxxxxxx>
Date: Tue, 8 Nov 2011 14:07:00 -0500
Cc: xfs@xxxxxxxxxxx
In-reply-to: <1320777990-6556-3-git-send-email-cmaiolino@xxxxxxxxxx>
References: <1320777990-6556-1-git-send-email-cmaiolino@xxxxxxxxxx> <1320777990-6556-3-git-send-email-cmaiolino@xxxxxxxxxx>
User-agent: Mutt/1.5.21 (2010-09-15)
On Tue, Nov 08, 2011 at 04:46:30PM -0200, Carlos Maiolino wrote:
> This patch makes mk_orphanage() to properly set the inode link count of
> the recently allocated inode in the AVL tree, avoiding the lost+found
> directory to be bypass the link count check in phase7 and possibly leaving
> lost+found directory with a wrong link count.

Thanks a lot, this looks very good technically.

Unfortunately I have a few style nipicks anyway:

> @@ -875,6 +877,18 @@ mk_orphanage(xfs_mount_t *mp)
>                       ORPHANAGE, error);
>       }
>       ip->i_d.di_nlink++;             /* account for . */
> +     ino = ip->i_ino;
> +
> +     irec = find_inode_rec(mp,
> +                     XFS_INO_TO_AGNO(mp, ino),
> +                     XFS_INO_TO_AGINO(mp,ino));

Please always use whitespaces after the comma.

> +     ino_offset = get_inode_offset(mp, ino, irec);
> +
> +     /* Set the inode allocated to lost+found as used in the AVL
> +      * tree, so it is not bypassed in phase 7
> +      */

The canonical comment style would be:

        /*
         * Mark the inode allocated to lost+found as used in the AVL tree
         * so it is not skipped in phase 7.
         */

(also note the slight change in wording to make it easier to parse for
 me, but I'm no native speaker either)
         *

> +     set_inode_used(irec,ino_offset);
> +     add_inode_ref(irec,ino_offset);

Same whitespace comment as above here.

Feel free to add my:

Reviewed-by: Christoph Hellwig <hch@xxxxxx>

after these cosmetic fixups.

<Prev in Thread] Current Thread [Next in Thread>