On Thu, Oct 27, 2016 at 08:15:40AM +1100, Dave Chinner wrote:
> On Wed, Oct 26, 2016 at 10:15:53AM -0700, Darrick J. Wong wrote:
> > On Wed, Oct 26, 2016 at 03:57:58AM -0700, Christoph Hellwig wrote:
> > > > --- a/repair/dinode.c
> > > > +++ b/repair/dinode.c
> > > > @@ -722,6 +722,9 @@ _("Fatal error: inode %" PRIu64 " - blkmap_set_ext(): %s\n"
> > > > * checking each entry without setting the
> > > > * block bitmap
> > > > */
> > > > + if (type == XR_INO_DATA &&
> > > > + xfs_sb_version_hasreflink(&mp->m_sb))
> > > > + goto skip_dup;
> > > > if (search_dup_extent(agno, agbno, ebno)) {
> > > > do_warn(
> > > > _("%s fork in ino %" PRIu64 " claims dup extent, "
> > > > @@ -731,6 +734,7 @@ _("%s fork in ino %" PRIu64 " claims dup extent, "
> > > > irec.br_blockcount);
> > > > goto done;
> > > > }
> > > > +skip_dup:
> > >
> > > For some weird reason this goto makes me sad. I'm getting into
> > > major nitpick terretory here, but can we avoid it? Either duplicate
> > > the *tot increment or find some funky condition?
> > >
> > > Otherwise this looks fine.
> >
> > Ugh, unnecessary goto. Sad! ;)
> >
> > But in all seriousness an 'else if' could do the same work, so:
> >
> > if (type == XR_INO_DATA &&
> > xfs_sb_version_hasreflink(&mp->m_sb))
> > ; /* avoid the dup extent check below */
> > else if (search_dup_extent(agno, agbno, ebno)) {
> > do_warn(...);
> > }
> > *tot += irec.br_blockcount;
>
> Even that is unnecessary, yes?
>
> if ((type != XR_INO_DATA ||
> !xfs_sb_version_hasreflink(&mp->m_sb)) &&
> search_dup_extent(agno, agbno, ebno) {
> do_warn(...);
> }
Err, yes.
--D
>
>
> Cheers,
>
> Dave.
> --
> Dave Chinner
> david@xxxxxxxxxxxxx
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
