xfs
[Top] [All Lists]

Re: [PATCH 12/12] xfs: Remove the macro XFS_BUFTARG_NAME

To: aelder@xxxxxxx
Subject: Re: [PATCH 12/12] xfs: Remove the macro XFS_BUFTARG_NAME
From: Chandra Seetharaman <sekharan@xxxxxxxxxx>
Date: Mon, 25 Jul 2011 15:18:42 -0700
Cc: Christoph Hellwig <hch@xxxxxxxxxxxxx>, xfs@xxxxxxxxxxx
In-reply-to: <1311609460.2914.25.camel@doink>
Organization: IBM
References: <20110722003226.21069.58401.sendpatchset@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx> <20110722003408.21069.44409.sendpatchset@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx> <1311364181.2771.114.camel@doink> <20110724113724.GC26332@xxxxxxxxxxxxx> <1311609460.2914.25.camel@doink>
Reply-to: sekharan@xxxxxxxxxx
On Mon, 2011-07-25 at 10:57 -0500, Alex Elder wrote:
> On Sun, 2011-07-24 at 07:37 -0400, Christoph Hellwig wrote:
> > On Fri, Jul 22, 2011 at 02:49:41PM -0500, Alex Elder wrote:
> > > On Thu, 2011-07-21 at 17:34 -0700, Chandra Seetharaman wrote:
> > > > Remove the definition and usages of the macro XFS_BUFTARG_NAME.
> > > > 
> > > > Signed-off-by: Chandra Seetharaman <sekharan@xxxxxxxxxx>
> > > > Reviewed-by: Christoph Hellwig <hch@xxxxxx>
> > > 
> > > 
> > > Wow, I hadn't looked at the definition of
> > > xfs_buf_target_name() before.  It's not safe
> > > (using a pointer to since-released stack space),
> > > though in practice it's going to be fine.
> > > 
> > > Defining it as an inline function with a static
> > > buffer would at least avoid that, though it
> > > means it's not reentrant either.
> > 
> > IMHO the right fix is to just kill it off entirely.  All XFS messages
> > now have the filesystem name prefixed to them, and while we can have
> > up to three devices, all these error messages can only hit either
> > the main or the log device, and it's obvious from the context which
> > one we did hit.
> 
> That's an even better idea.  I was only reacting to the
> code in front of me, but yes, removing it entirely
> would be good.
> 
> For now though, I intend to commit this (in its now updated
> form).  It can be removed as a separate patch.

I will do it.

chandra
> 
>                                       -Alex
> 


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