xfs
[Top] [All Lists]

Re: [PATCH 5/9] repair: detect CRC errors in AG headers

To: Dave Chinner <david@xxxxxxxxxxxxx>
Subject: Re: [PATCH 5/9] repair: detect CRC errors in AG headers
From: Christoph Hellwig <hch@xxxxxxxxxxxxx>
Date: Thu, 24 Apr 2014 22:55:16 -0700
Cc: xfs@xxxxxxxxxxx
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <1398315722-20870-6-git-send-email-david@xxxxxxxxxxxxx>
References: <1398315722-20870-1-git-send-email-david@xxxxxxxxxxxxx> <1398315722-20870-6-git-send-email-david@xxxxxxxxxxxxx>
User-agent: Mutt/1.5.21 (2010-09-15)
>       sb = (xfs_sb_t *)calloc(BBSIZE, 1);

If you already do various cosmetic changes I'd recommend removing the
useles case here as well.

> -     if (status & XR_AG_AGF)  {
> +     if (agf_dirty || status & XR_AG_AGF)  {

> -     if (status & XR_AG_AGI)  {
> +     if (agi_dirty || status & XR_AG_AGI)  {

I can't see how agf_dirty and agi_dirty would ever be set at this point.

> +out_free:
> +     if (sb)
> +             free(sb);
> +     if (agibuf)
> +             libxfs_putbuf(agibuf);
> +     if (agfbuf)
> +             libxfs_putbuf(agfbuf);
> +     if (sbbuf)
> +             libxfs_putbuf(sbbuf);
> +     if (objname)
> +             do_error(_("can't get %s for ag %d\n"), objname, agno);
> +     return;

No need for a return statement at the end of a void returning function.

Also any reason for not using one goto for each unwind step like we do
elsewhere instead of the if (!NULL) checks?

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