Linux-XFS Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH v2 0/2] xfs: Fix compiler warnings
@ 2020-07-27  2:26 Allison Collins
  2020-07-27  2:26 ` [PATCH v2 1/2] xfs: Fix compiler warning in xfs_attr_node_removename_setup Allison Collins
  2020-07-27  2:26 ` [PATCH v2 2/2] xfs: Fix compiler warning in xfs_attr_shortform_add Allison Collins
  0 siblings, 2 replies; 8+ messages in thread
From: Allison Collins @ 2020-07-27  2:26 UTC (permalink / raw)
  To: linux-xfs

Some variables added in the delayed attr series were only used in ASSERT
checks.  This caused some compiler warnings.  This is only declare them when
DEBUG is set

Allison Collins (2):
  xfs: Fix compiler warning in xfs_attr_node_removename_setup
  xfs: Fix compiler warning in xfs_attr_shortform_add

 fs/xfs/libxfs/xfs_attr.c      | 2 ++
 fs/xfs/libxfs/xfs_attr_leaf.c | 5 ++++-
 2 files changed, 6 insertions(+), 1 deletion(-)

-- 
2.7.4


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v2 1/2] xfs: Fix compiler warning in xfs_attr_node_removename_setup
  2020-07-27  2:26 [PATCH v2 0/2] xfs: Fix compiler warnings Allison Collins
@ 2020-07-27  2:26 ` Allison Collins
  2020-07-27 15:46   ` Darrick J. Wong
  2020-07-27  2:26 ` [PATCH v2 2/2] xfs: Fix compiler warning in xfs_attr_shortform_add Allison Collins
  1 sibling, 1 reply; 8+ messages in thread
From: Allison Collins @ 2020-07-27  2:26 UTC (permalink / raw)
  To: linux-xfs

Fix compiler warning for variable 'blk' set but not used in
xfs_attr_node_removename_setup.  blk is used only in an ASSERT so only
declare blk when DEBUG is set.

Reported-by: kernel test robot <lkp@intel.com>
Signed-off-by: Allison Collins <allison.henderson@oracle.com>
---
 fs/xfs/libxfs/xfs_attr.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c
index d4583a0..5168d32 100644
--- a/fs/xfs/libxfs/xfs_attr.c
+++ b/fs/xfs/libxfs/xfs_attr.c
@@ -1174,7 +1174,9 @@ int xfs_attr_node_removename_setup(
 	struct xfs_da_state	**state)
 {
 	int			error;
+#ifdef DEBUG
 	struct xfs_da_state_blk	*blk;
+#endif
 
 	error = xfs_attr_node_hasname(args, state);
 	if (error != -EEXIST)
-- 
2.7.4


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v2 2/2] xfs: Fix compiler warning in xfs_attr_shortform_add
  2020-07-27  2:26 [PATCH v2 0/2] xfs: Fix compiler warnings Allison Collins
  2020-07-27  2:26 ` [PATCH v2 1/2] xfs: Fix compiler warning in xfs_attr_node_removename_setup Allison Collins
@ 2020-07-27  2:26 ` Allison Collins
  1 sibling, 0 replies; 8+ messages in thread
From: Allison Collins @ 2020-07-27  2:26 UTC (permalink / raw)
  To: linux-xfs

Fix compiler warning warning: variable 'error' set but not used in
xfs_attr_shortform_add. error is used only in an ASSERT so only declare
error when DEBUG is set.

Reported-by: kernel test robot <lkp@intel.com>
Signed-off-by: Allison Collins <allison.henderson@oracle.com>
---
 fs/xfs/libxfs/xfs_attr_leaf.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/fs/xfs/libxfs/xfs_attr_leaf.c b/fs/xfs/libxfs/xfs_attr_leaf.c
index ad7b351..db985b8 100644
--- a/fs/xfs/libxfs/xfs_attr_leaf.c
+++ b/fs/xfs/libxfs/xfs_attr_leaf.c
@@ -715,7 +715,10 @@ xfs_attr_shortform_add(
 {
 	struct xfs_attr_shortform	*sf;
 	struct xfs_attr_sf_entry	*sfe;
-	int				offset, size, error;
+	int				offset, size;
+#if DEBUG
+	int				error;
+#endif
 	struct xfs_mount		*mp;
 	struct xfs_inode		*dp;
 	struct xfs_ifork		*ifp;
-- 
2.7.4


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 1/2] xfs: Fix compiler warning in xfs_attr_node_removename_setup
  2020-07-27  2:26 ` [PATCH v2 1/2] xfs: Fix compiler warning in xfs_attr_node_removename_setup Allison Collins
@ 2020-07-27 15:46   ` Darrick J. Wong
  2020-07-27 16:42     ` Allison Collins
  2020-07-27 16:51     ` Allison Collins
  0 siblings, 2 replies; 8+ messages in thread
From: Darrick J. Wong @ 2020-07-27 15:46 UTC (permalink / raw)
  To: Allison Collins; +Cc: linux-xfs

On Sun, Jul 26, 2020 at 07:26:07PM -0700, Allison Collins wrote:
> Fix compiler warning for variable 'blk' set but not used in
> xfs_attr_node_removename_setup.  blk is used only in an ASSERT so only
> declare blk when DEBUG is set.
> 
> Reported-by: kernel test robot <lkp@intel.com>
> Signed-off-by: Allison Collins <allison.henderson@oracle.com>
> ---
>  fs/xfs/libxfs/xfs_attr.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c
> index d4583a0..5168d32 100644
> --- a/fs/xfs/libxfs/xfs_attr.c
> +++ b/fs/xfs/libxfs/xfs_attr.c
> @@ -1174,7 +1174,9 @@ int xfs_attr_node_removename_setup(
>  	struct xfs_da_state	**state)
>  {
>  	int			error;
> +#ifdef DEBUG
>  	struct xfs_da_state_blk	*blk;
> +#endif

But now a non-DEBUG compilation will trip over the assignment to blk:

	blk = &(*state)->path.blk[(*state)->path.active - 1];

that comes just before the asserts, right?

	ASSERT((*state)->path.blk[(*state)->path.active - 1].bp != NULL);
	ASSERT((*state)->path.blk[(*state)->path.active - 1].magic ==
		XFS_ATTR_LEAF_MAGIC);

In the end you probably just want to encode the accessor logic in the
assert body so the whole thing just disappears entirely.

--D

>  
>  	error = xfs_attr_node_hasname(args, state);
>  	if (error != -EEXIST)
> -- 
> 2.7.4
> 

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 1/2] xfs: Fix compiler warning in xfs_attr_node_removename_setup
  2020-07-27 15:46   ` Darrick J. Wong
@ 2020-07-27 16:42     ` Allison Collins
  2020-07-27 16:51     ` Allison Collins
  1 sibling, 0 replies; 8+ messages in thread
From: Allison Collins @ 2020-07-27 16:42 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: linux-xfs



On 7/27/20 8:46 AM, Darrick J. Wong wrote:
> On Sun, Jul 26, 2020 at 07:26:07PM -0700, Allison Collins wrote:
>> Fix compiler warning for variable 'blk' set but not used in
>> xfs_attr_node_removename_setup.  blk is used only in an ASSERT so only
>> declare blk when DEBUG is set.
>>
>> Reported-by: kernel test robot <lkp@intel.com>
>> Signed-off-by: Allison Collins <allison.henderson@oracle.com>
>> ---
>>   fs/xfs/libxfs/xfs_attr.c | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c
>> index d4583a0..5168d32 100644
>> --- a/fs/xfs/libxfs/xfs_attr.c
>> +++ b/fs/xfs/libxfs/xfs_attr.c
>> @@ -1174,7 +1174,9 @@ int xfs_attr_node_removename_setup(
>>   	struct xfs_da_state	**state)
>>   {
>>   	int			error;
>> +#ifdef DEBUG
>>   	struct xfs_da_state_blk	*blk;
>> +#endif
> 
> But now a non-DEBUG compilation will trip over the assignment to blk:
> 
> 	blk = &(*state)->path.blk[(*state)->path.active - 1];
> 
> that comes just before the asserts, right?
> 
> 	ASSERT((*state)->path.blk[(*state)->path.active - 1].bp != NULL);
> 	ASSERT((*state)->path.blk[(*state)->path.active - 1].magic ==
> 		XFS_ATTR_LEAF_MAGIC);
> 
> In the end you probably just want to encode the accessor logic in the
> assert body so the whole thing just disappears entirely.
Alrighty, will fix

Allison
> 
> --D
> 
>>   
>>   	error = xfs_attr_node_hasname(args, state);
>>   	if (error != -EEXIST)
>> -- 
>> 2.7.4
>>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 1/2] xfs: Fix compiler warning in xfs_attr_node_removename_setup
  2020-07-27 15:46   ` Darrick J. Wong
  2020-07-27 16:42     ` Allison Collins
@ 2020-07-27 16:51     ` Allison Collins
  2020-07-27 18:50       ` Darrick J. Wong
  1 sibling, 1 reply; 8+ messages in thread
From: Allison Collins @ 2020-07-27 16:51 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: linux-xfs



On 7/27/20 8:46 AM, Darrick J. Wong wrote:
> On Sun, Jul 26, 2020 at 07:26:07PM -0700, Allison Collins wrote:
>> Fix compiler warning for variable 'blk' set but not used in
>> xfs_attr_node_removename_setup.  blk is used only in an ASSERT so only
>> declare blk when DEBUG is set.
>>
>> Reported-by: kernel test robot <lkp@intel.com>
>> Signed-off-by: Allison Collins <allison.henderson@oracle.com>
>> ---
>>   fs/xfs/libxfs/xfs_attr.c | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c
>> index d4583a0..5168d32 100644
>> --- a/fs/xfs/libxfs/xfs_attr.c
>> +++ b/fs/xfs/libxfs/xfs_attr.c
>> @@ -1174,7 +1174,9 @@ int xfs_attr_node_removename_setup(
>>   	struct xfs_da_state	**state)
>>   {
>>   	int			error;
>> +#ifdef DEBUG
>>   	struct xfs_da_state_blk	*blk;
>> +#endif
> 
> But now a non-DEBUG compilation will trip over the assignment to blk:
> 
> 	blk = &(*state)->path.blk[(*state)->path.active - 1];
> 
> that comes just before the asserts, right?
> 
> 	ASSERT((*state)->path.blk[(*state)->path.active - 1].bp != NULL);
> 	ASSERT((*state)->path.blk[(*state)->path.active - 1].magic ==
> 		XFS_ATTR_LEAF_MAGIC);
> 
> In the end you probably just want to encode the accessor logic in the
> assert body so the whole thing just disappears entirely.
Are you sure you'd rather have it that way, then once up in the 
declaration?  Like this:

#ifdef DEBUG
	struct xfs_da_state_blk	*blk = 
&(*state)->path.blk[(*state)->path.active - 1];
#endif

> 
> --D
> 
>>   
>>   	error = xfs_attr_node_hasname(args, state);
>>   	if (error != -EEXIST)
>> -- 
>> 2.7.4
>>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 1/2] xfs: Fix compiler warning in xfs_attr_node_removename_setup
  2020-07-27 16:51     ` Allison Collins
@ 2020-07-27 18:50       ` Darrick J. Wong
  2020-07-27 19:20         ` Allison Collins
  0 siblings, 1 reply; 8+ messages in thread
From: Darrick J. Wong @ 2020-07-27 18:50 UTC (permalink / raw)
  To: Allison Collins; +Cc: linux-xfs

On Mon, Jul 27, 2020 at 09:51:54AM -0700, Allison Collins wrote:
> 
> 
> On 7/27/20 8:46 AM, Darrick J. Wong wrote:
> > On Sun, Jul 26, 2020 at 07:26:07PM -0700, Allison Collins wrote:
> > > Fix compiler warning for variable 'blk' set but not used in
> > > xfs_attr_node_removename_setup.  blk is used only in an ASSERT so only
> > > declare blk when DEBUG is set.
> > > 
> > > Reported-by: kernel test robot <lkp@intel.com>
> > > Signed-off-by: Allison Collins <allison.henderson@oracle.com>
> > > ---
> > >   fs/xfs/libxfs/xfs_attr.c | 2 ++
> > >   1 file changed, 2 insertions(+)
> > > 
> > > diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c
> > > index d4583a0..5168d32 100644
> > > --- a/fs/xfs/libxfs/xfs_attr.c
> > > +++ b/fs/xfs/libxfs/xfs_attr.c
> > > @@ -1174,7 +1174,9 @@ int xfs_attr_node_removename_setup(
> > >   	struct xfs_da_state	**state)
> > >   {
> > >   	int			error;
> > > +#ifdef DEBUG
> > >   	struct xfs_da_state_blk	*blk;
> > > +#endif
> > 
> > But now a non-DEBUG compilation will trip over the assignment to blk:
> > 
> > 	blk = &(*state)->path.blk[(*state)->path.active - 1];
> > 
> > that comes just before the asserts, right?
> > 
> > 	ASSERT((*state)->path.blk[(*state)->path.active - 1].bp != NULL);
> > 	ASSERT((*state)->path.blk[(*state)->path.active - 1].magic ==
> > 		XFS_ATTR_LEAF_MAGIC);
> > 
> > In the end you probably just want to encode the accessor logic in the
> > assert body so the whole thing just disappears entirely.
> Are you sure you'd rather have it that way, then once up in the declaration?
> Like this:
> 
> #ifdef DEBUG
> 	struct xfs_da_state_blk	*blk = &(*state)->path.blk[(*state)->path.active -
> 1];
> #endif

I thought xfs_attr_node_hasname could allocate the da state and set
*state, which means that we can't dereference *state until after that
call?

--D

> > 
> > --D
> > 
> > >   	error = xfs_attr_node_hasname(args, state);
> > >   	if (error != -EEXIST)
> > > -- 
> > > 2.7.4
> > > 

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 1/2] xfs: Fix compiler warning in xfs_attr_node_removename_setup
  2020-07-27 18:50       ` Darrick J. Wong
@ 2020-07-27 19:20         ` Allison Collins
  0 siblings, 0 replies; 8+ messages in thread
From: Allison Collins @ 2020-07-27 19:20 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: linux-xfs



On 7/27/20 11:50 AM, Darrick J. Wong wrote:
> On Mon, Jul 27, 2020 at 09:51:54AM -0700, Allison Collins wrote:
>>
>>
>> On 7/27/20 8:46 AM, Darrick J. Wong wrote:
>>> On Sun, Jul 26, 2020 at 07:26:07PM -0700, Allison Collins wrote:
>>>> Fix compiler warning for variable 'blk' set but not used in
>>>> xfs_attr_node_removename_setup.  blk is used only in an ASSERT so only
>>>> declare blk when DEBUG is set.
>>>>
>>>> Reported-by: kernel test robot <lkp@intel.com>
>>>> Signed-off-by: Allison Collins <allison.henderson@oracle.com>
>>>> ---
>>>>    fs/xfs/libxfs/xfs_attr.c | 2 ++
>>>>    1 file changed, 2 insertions(+)
>>>>
>>>> diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c
>>>> index d4583a0..5168d32 100644
>>>> --- a/fs/xfs/libxfs/xfs_attr.c
>>>> +++ b/fs/xfs/libxfs/xfs_attr.c
>>>> @@ -1174,7 +1174,9 @@ int xfs_attr_node_removename_setup(
>>>>    	struct xfs_da_state	**state)
>>>>    {
>>>>    	int			error;
>>>> +#ifdef DEBUG
>>>>    	struct xfs_da_state_blk	*blk;
>>>> +#endif
>>>
>>> But now a non-DEBUG compilation will trip over the assignment to blk:
>>>
>>> 	blk = &(*state)->path.blk[(*state)->path.active - 1];
>>>
>>> that comes just before the asserts, right?
>>>
>>> 	ASSERT((*state)->path.blk[(*state)->path.active - 1].bp != NULL);
>>> 	ASSERT((*state)->path.blk[(*state)->path.active - 1].magic ==
>>> 		XFS_ATTR_LEAF_MAGIC);
>>>
>>> In the end you probably just want to encode the accessor logic in the
>>> assert body so the whole thing just disappears entirely.
>> Are you sure you'd rather have it that way, then once up in the declaration?
>> Like this:
>>
>> #ifdef DEBUG
>> 	struct xfs_da_state_blk	*blk = &(*state)->path.blk[(*state)->path.active -
>> 1];
>> #endif
> 
> I thought xfs_attr_node_hasname could allocate the da state and set
> *state, which means that we can't dereference *state until after that
> call?
I see, ok then, will add the deference in the asserts

Allison

> 
> --D
> 
>>>
>>> --D
>>>
>>>>    	error = xfs_attr_node_hasname(args, state);
>>>>    	if (error != -EEXIST)
>>>> -- 
>>>> 2.7.4
>>>>

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, back to index

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-27  2:26 [PATCH v2 0/2] xfs: Fix compiler warnings Allison Collins
2020-07-27  2:26 ` [PATCH v2 1/2] xfs: Fix compiler warning in xfs_attr_node_removename_setup Allison Collins
2020-07-27 15:46   ` Darrick J. Wong
2020-07-27 16:42     ` Allison Collins
2020-07-27 16:51     ` Allison Collins
2020-07-27 18:50       ` Darrick J. Wong
2020-07-27 19:20         ` Allison Collins
2020-07-27  2:26 ` [PATCH v2 2/2] xfs: Fix compiler warning in xfs_attr_shortform_add Allison Collins

Linux-XFS Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-xfs/0 linux-xfs/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-xfs linux-xfs/ https://lore.kernel.org/linux-xfs \
		linux-xfs@vger.kernel.org
	public-inbox-index linux-xfs

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-xfs


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git