Re: [PATCH v3 4/4] xfs: remove the xfs_qoff_logitem_t typedef

[Dave Chinner <david@fromorbit.com>]

On Sun, Nov 10, 2019 at 07:24:04AM +0100, Pavel Reichl wrote:
> Signed-off-by: Pavel Reichl <preichl@redhat.com>
> ---
>  fs/xfs/libxfs/xfs_trans_resv.c |  4 ++--
>  fs/xfs/xfs_dquot_item.h        | 28 +++++++++++++++-------------
>  fs/xfs/xfs_qm_syscalls.c       | 29 ++++++++++++++++-------------
>  fs/xfs/xfs_trans_dquot.c       | 14 +++++++-------
>  4 files changed, 40 insertions(+), 35 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_trans_resv.c b/fs/xfs/libxfs/xfs_trans_resv.c
> index 271cca13565b..eb7fe42b1d61 100644
> --- a/fs/xfs/libxfs/xfs_trans_resv.c
> +++ b/fs/xfs/libxfs/xfs_trans_resv.c
> @@ -741,7 +741,7 @@ xfs_calc_qm_dqalloc_reservation(
>  
>  /*
>   * Turning off quotas.
> - *    the xfs_qoff_logitem_t: sizeof(struct xfs_qoff_logitem) * 2
> + *    sizeof(struct xfs_qoff_logitem) * 2

This should be

 *    the quota off logitems: sizeof(struct xfs_qoff_logitem) * 2

>   *    the superblock for the quota flags: sector size
>   */
>  STATIC uint
> @@ -754,7 +754,7 @@ xfs_calc_qm_quotaoff_reservation(
>  
>  /*
>   * End of turning off quotas.
> - *    the xfs_qoff_logitem_t: sizeof(struct xfs_qoff_logitem) * 2
> + *    sizeof(struct xfs_qoff_logitem) * 2

Ditto.

>   */
>  STATIC uint
>  xfs_calc_qm_quotaoff_end_reservation(void)
> diff --git a/fs/xfs/xfs_dquot_item.h b/fs/xfs/xfs_dquot_item.h
> index 3a64a7fd3b8a..e94003271e74 100644
> --- a/fs/xfs/xfs_dquot_item.h
> +++ b/fs/xfs/xfs_dquot_item.h
> @@ -12,24 +12,26 @@ struct xfs_mount;
>  struct xfs_qoff_logitem;
>  
>  struct xfs_dq_logitem {
> -	struct xfs_log_item	 qli_item;	/* common portion */
> +	struct xfs_log_item	qli_item;	/* common portion */
>  	struct xfs_dquot	*qli_dquot;	/* dquot ptr */
> -	xfs_lsn_t		 qli_flush_lsn;	/* lsn at last flush */
> +	xfs_lsn_t		qli_flush_lsn;	/* lsn at last flush */
>  };
>  
> -typedef struct xfs_qoff_logitem {
> -	struct xfs_log_item	 qql_item;	/* common portion */
> -	struct xfs_qoff_logitem *qql_start_lip; /* qoff-start logitem, if any */
> +struct xfs_qoff_logitem {
> +	struct xfs_log_item	qql_item;	/* common portion */
> +	struct xfs_qoff_logitem *qql_start_lip;	/* qoff-start logitem, if any */
>  	unsigned int		qql_flags;
> -} xfs_qoff_logitem_t;
> +};
>  
>  
> -extern void		   xfs_qm_dquot_logitem_init(struct xfs_dquot *);
> -extern xfs_qoff_logitem_t *xfs_qm_qoff_logitem_init(struct xfs_mount *,
> -					struct xfs_qoff_logitem *, uint);
> -extern xfs_qoff_logitem_t *xfs_trans_get_qoff_item(struct xfs_trans *,
> -					struct xfs_qoff_logitem *, uint);
> -extern void		   xfs_trans_log_quotaoff_item(struct xfs_trans *,
> -					struct xfs_qoff_logitem *);
> +void			xfs_qm_dquot_logitem_init(struct xfs_dquot *dqp);
> +struct xfs_qoff_logitem	*xfs_qm_qoff_logitem_init(struct xfs_mount *mp,
> +					struct xfs_qoff_logitem *start,
> +					uint flags);
> +struct xfs_qoff_logitem	*xfs_trans_get_qoff_item(struct xfs_trans *tp,
> +					struct xfs_qoff_logitem *startqoff,
> +					uint flags);
> +void			xfs_trans_log_quotaoff_item(struct xfs_trans *tp,
> +					struct xfs_qoff_logitem *qlp);

I'd get rid of the tabs completely here:

void xfs_qm_dquot_logitem_init(struct xfs_dquot *dqp);
struct xfs_qoff_logitem *xfs_qm_qoff_logitem_init(struct xfs_mount *mp,
				struct xfs_qoff_logitem *start, uint flags);
struct xfs_qoff_logitem *xfs_trans_get_qoff_item(struct xfs_trans *tp,
				struct xfs_qoff_logitem *startqoff, uint flags);
void xfs_trans_log_quotaoff_item(struct xfs_trans *tp,
				struct xfs_qoff_logitem *qlp);

>  
>  #endif	/* __XFS_DQUOT_ITEM_H__ */
> diff --git a/fs/xfs/xfs_qm_syscalls.c b/fs/xfs/xfs_qm_syscalls.c
> index da7ad0383037..e685b9ae90b9 100644
> --- a/fs/xfs/xfs_qm_syscalls.c
> +++ b/fs/xfs/xfs_qm_syscalls.c
> @@ -19,9 +19,12 @@
>  #include "xfs_qm.h"
>  #include "xfs_icache.h"
>  
> -STATIC int	xfs_qm_log_quotaoff(xfs_mount_t *, xfs_qoff_logitem_t **, uint);
> -STATIC int	xfs_qm_log_quotaoff_end(xfs_mount_t *, xfs_qoff_logitem_t *,
> -					uint);
> +STATIC int xfs_qm_log_quotaoff(struct xfs_mount *mp,
> +					struct xfs_qoff_logitem **qoffstartp,
> +					uint flags);
> +STATIC int xfs_qm_log_quotaoff_end(struct xfs_mount *mp,
> +					struct xfs_qoff_logitem *startqoff,
> +					uint flags);

And pull the indent back here.

STATIC int xfs_qm_log_quotaoff(struct xfs_mount *mp,
			struct xfs_qoff_logitem **qoffstartp, uint flags);
STATIC int xfs_qm_log_quotaoff_end(struct xfs_mount *mp,
			struct xfs_qoff_logitem *startqoff, uint flags);

Actaully, no, what would be better is another patch to move the
xfs_qm_log_quotaoff() and xfs_qm_log_quotaoff_end() functions above
the code that calls them, so the need for forward prototypes goes
away completely. i.e. reformat in one patch, move in another (or the
other way around). Code movement patches shouldn't make any
other changes in them.


> @@ -568,13 +571,13 @@ xfs_qm_log_quotaoff_end(
>  
>  STATIC int
>  xfs_qm_log_quotaoff(
> -	xfs_mount_t	       *mp,
> -	xfs_qoff_logitem_t     **qoffstartp,
> -	uint		       flags)
> +	struct xfs_mount	*mp,
> +	struct xfs_qoff_logitem	**qoffstartp,
> +	uint			flags)
>  {
> -	xfs_trans_t	       *tp;
> +	struct xfs_trans	*tp;
>  	int			error;
> -	xfs_qoff_logitem_t     *qoffi;
> +	struct xfs_qoff_logitem	*qoffi;
>  
>  	*qoffstartp = NULL;
>  
> diff --git a/fs/xfs/xfs_trans_dquot.c b/fs/xfs/xfs_trans_dquot.c
> index d319347093d6..0fdc96ed805a 100644
> --- a/fs/xfs/xfs_trans_dquot.c
> +++ b/fs/xfs/xfs_trans_dquot.c
> @@ -824,13 +824,13 @@ xfs_trans_reserve_quota_nblks(
>  /*
>   * This routine is called to allocate a quotaoff log item.
>   */
> -xfs_qoff_logitem_t *
> +struct xfs_qoff_logitem *
>  xfs_trans_get_qoff_item(
> -	xfs_trans_t		*tp,
> -	xfs_qoff_logitem_t	*startqoff,
> -	uint			flags)
> +	struct xfs_trans		*tp,
> +	struct xfs_qoff_logitem	*startqoff,
> +	uint			 flags)

That doesn't look right. Extra tab before *tp, extra space
before flags.

FWIW, I use this rule in vim to highlight stray whitespace:

" highlight whitespace damage
highlight RedundantSpaces ctermbg=red guibg=red
match RedundantSpaces /\s\+$\| \+\ze\t/

Modify the regex to suit the whitespace damage you want it to
find...

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com