xfs
[Top] [All Lists]

Re: [PATCH] xfs: rename log structure and remove xlog_t typedef.

To: Mark Tinguely <tinguely@xxxxxxx>
Subject: Re: [PATCH] xfs: rename log structure and remove xlog_t typedef.
From: Dave Chinner <david@xxxxxxxxxxxxx>
Date: Wed, 13 Jun 2012 12:28:15 +1000
Cc: xfs-oss <xfs@xxxxxxxxxxx>
In-reply-to: <4FD78236.6010109@xxxxxxx>
References: <4FD78236.6010109@xxxxxxx>
User-agent: Mutt/1.5.21 (2010-09-15)
On Tue, Jun 12, 2012 at 12:53:58PM -0500, Mark Tinguely wrote:
> From: Mark Tinguely <tinguely@xxxxxxx>
> 
> Rename the log structure to xlog and drop the xlog_t type definition.
> This will make it easier to distinguish the XFS log from other logs
> in Linux.
> 
> Signed-off-by: Mark Tinguely <tinguely@xxxxxxx>

Couple of things - it looks like you mailer has line wrapped the
patch. When you repost it, can you make sure it isn't line wrapped?

> ---
>  fs/xfs/xfs_log.c         |  130
> +++++++++++++++++++++++------------------------

e.g. That's a linewrapped line....

> @@ -1055,7 +1055,7 @@ xlog_iodone(xfs_buf_t *bp)
> 
>  STATIC void
>  xlog_get_iclog_buffer_size(xfs_mount_t       *mp,
> -                        xlog_t       *log)
> +                        struct xlog  *log)
>  {
>       int size;
>       int xhdrs;

When we modify function headers in non-standard formats, we tend to
convert them at the same time to the standard format and remove all
the typedefs. i.e:

 STATIC void
-xlog_get_iclog_buffer_size(xfs_mount_t *mp,
-                          xlog_t       *log)
+xlog_get_iclog_buffer_size(
+       struct xfs_mount        *mp,
+       struct xlog             *log)
 {

Note that I'm only suggesting removing all the other typedefs for the
functions that you need to reformat, not every single one that has a
xlog_t->struct xlog transformation. ;)

The log is one of the few places left that has non-standard function
formats, so can you go through the patch and convert all the
functions that are touched and convert them fully? This way we
slowly improve the code as we go, and seeing as this is pretty much
a cleanup patch it makes sense to do this at the same time....

Otherwise it looks fine. Thanks for taking a shot at this, Mark.

Cheers,

Dave.
-- 
Dave Chinner
david@xxxxxxxxxxxxx

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