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;
--D
--
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
