* [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 related [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
* [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 related [flat|nested] 8+ messages in thread
end of thread, other threads:[~2020-07-27 19:22 UTC | newest] 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
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).