On Mon, Dec 07, 2020 at 10:19:48PM +0800, Gao Xiang wrote:
> On Mon, Dec 07, 2020 at 02:49:41PM +0100, Christoph Hellwig wrote:
> > On Mon, Dec 07, 2020 at 08:15:30AM +0800, Gao Xiang wrote:
> > > /*
> > > + * Initialise a newly allocated inode and return the in-core inode to the
> > > + * caller locked exclusively.
> > > */
> > > -static int
> > > -xfs_ialloc(
> > > - xfs_trans_t *tp,
> > > - xfs_inode_t *pip,
> > > - umode_t mode,
> > > - xfs_nlink_t nlink,
> > > - dev_t rdev,
> > > - prid_t prid,
> > > - xfs_buf_t **ialloc_context,
> > > - xfs_inode_t **ipp)
> > > +static struct xfs_inode *
> > > +xfs_dir_ialloc_init(
> >
> > This is boderline bikeshedding, but I would just call this
> > xfs_init_new_inode.
>
> (See below...)
>
> >
> > > int
> > > xfs_dir_ialloc(
> > > @@ -954,83 +908,59 @@ xfs_dir_ialloc(
> > > xfs_inode_t **ipp) /* pointer to inode; it will be
> > > locked. */
> > > {
> > > xfs_inode_t *ip;
> > > xfs_buf_t *ialloc_context = NULL;
> > > + xfs_ino_t pino = dp ? dp->i_ino : 0;
> >
> > Maybe spell out parent_inode? pino reminds of some of the weird Windows
> > code that start all variable names for pointers with a "p".
>
> Ok, yet pino is somewhat common, as I saw it in f2fs and jffs2 before.
> I know you mean 'Hungarian naming conventions'.
>
> If you don't like pino. How about parent_ino? since parent_inode occurs me
> about "struct inode *" or something like this (a pointer around some inode),
> rather than an inode number.
>
> >
> > > + /* Initialise the newly allocated inode. */
> > > + ip = xfs_dir_ialloc_init(*tpp, dp, ino, mode, nlink, rdev, prid);
> > > + if (IS_ERR(ip))
> > > + return PTR_ERR(ip);
> > > + *ipp = ip;
> > > return 0;
> >
> > I wonder if we should just return the inode by reference from
> > xfs_dir_ialloc_init as well, as that nicely fits the calling convention
> > in the caller, i.e. this could become
> >
> > return xfs_init_new_inode(*tpp, dp, ino, mode, nlink, rdev, prid, ipp);
> >
> > Note with the right naming we don't really need the comment either,
> > as the function name should explain everything.
>
> Okay, the name was from Dave to unify the prefix (namespace)... I think it'd
> be better to get Dave's idea about this as well. As of me, I'm fine with
> either way.
I'm fine with that.
-Dave.
--
Dave Chinner
david@xxxxxxxxxxxxx
