All of lore.kernel.org
 help / color / mirror / Atom feed
From: Eric Sandeen <sandeen@sandeen.net>
To: Li Zhong <zhong@linux.vnet.ibm.com>
Cc: Chandra Seetharaman <sekharan@us.ibm.com>,
	Mark Tinguely <tinguely@sgi.com>, xfsprogs <xfs@oss.sgi.com>
Subject: Re: [PATCH 2/2] xfsprogs: fix return value of verify_set_primary_sb()
Date: Thu, 26 Sep 2013 09:43:18 -0500	[thread overview]
Message-ID: <52444806.6020900@sandeen.net> (raw)
In-Reply-To: <1380178092.2983.14.camel@ThinkPad-T5421>

On 9/26/13 1:48 AM, Li Zhong wrote:
> If get_sb() fails because of EOF, it will return with retval 1, which will
> then be interpreted as XR_BAD_MAGIC("bad magic number") in phase1() when
> warning the user.
> 
> This patch fix it by using XR_EOF here, so it would be interpreted correctly.
> Also change the associated comments about the return value.
> 
> Signed-off-by: Li Zhong <zhong@linux.vnet.ibm.com>

I think this is OK.  I had originally thought we should return whatever XR_*
code get_sb() returned, but I wasn't thinking straight; we ignore those
other codes, and only stop if we see XR_EOF.

So this is fine as far as it goes; I noticed something else in the process
though so I'll send a patch for that.  :)

Reviewed-by: Eric Sandeen <sandeen@redhat.com>

> ---
>  repair/sb.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/repair/sb.c b/repair/sb.c
> index d34d7a2..2e35a4c 100644
> --- a/repair/sb.c
> +++ b/repair/sb.c
> @@ -665,7 +665,7 @@ get_sb_geometry(fs_geometry_t *geo, xfs_sb_t *sbp)
>   * primary and compare the geometries in the secondaries against
>   * the geometry indicated by the primary.
>   *
> - * returns 1 if bad, 0 if ok
> + * returns 0 if ok, else error code (XR_EOF, XR_INSUFF_SEC_SB, etc).
>   */
>  int
>  verify_set_primary_sb(xfs_sb_t		*rsb,
> @@ -732,7 +732,7 @@ verify_set_primary_sb(xfs_sb_t		*rsb,
>  			checked[agno] = 1;
>  
>  			if (get_sb(sb, off, size, agno) == XR_EOF)  {
> -				retval = 1;
> +				retval = XR_EOF;
>  				goto out_free_list;
>  			}
>  
> 

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

  reply	other threads:[~2013-09-26 14:43 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-09-22  6:01 [PATCH] xfsprogs: fix potential memory leak in verify_set_primary_sb() Li Zhong
2013-09-24 18:59 ` Mark Tinguely
2013-09-25  7:32   ` [PATCH v2] " Li Zhong
2013-09-25 13:26     ` Mark Tinguely
2013-09-25 14:28     ` Eric Sandeen
2013-09-26  6:41       ` Li Zhong
2013-09-26  6:45       ` [PATCH v3 1/2] " Li Zhong
2013-09-26  6:48         ` [PATCH 2/2] xfsprogs: fix return value of verify_set_primary_sb() Li Zhong
2013-09-26 14:43           ` Eric Sandeen [this message]
2013-10-18 16:42           ` Rich Johnston
2013-09-26 14:31         ` [PATCH v3 1/2] xfsprogs: fix potential memory leak in verify_set_primary_sb() Eric Sandeen
2013-09-27  3:05           ` Li Zhong
2013-09-27  3:24             ` Eric Sandeen
2013-09-27  5:24               ` Li Zhong
2013-10-18 16:40         ` Rich Johnston
2013-09-25  7:34   ` [PATCH] " Li Zhong

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=52444806.6020900@sandeen.net \
    --to=sandeen@sandeen.net \
    --cc=sekharan@us.ibm.com \
    --cc=tinguely@sgi.com \
    --cc=xfs@oss.sgi.com \
    --cc=zhong@linux.vnet.ibm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.