Re: [PATCH 04/14] repair: split up scanfunc_ino

[Eric Sandeen <sandeen@sandeen.net>]

Christoph Hellwig wrote:


> Split out a helper to scan a single inode chunk for suspect inodes from
> scanfunc_ino to make it more readable.
>
>
> Signed-off-by: Barry Naujok <bnaujok@sgi.com>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
>
> Index: xfsprogs-dev/repair/scan.c
> ===================================================================
> --- xfsprogs-dev.orig/repair/scan.c	2009-08-21 19:00:15.000000000 +0000
> +++ xfsprogs-dev/repair/scan.c	2009-08-21 19:03:26.000000000 +0000
>
...

> +
> +	/*
> +	 * set state of each block containing inodes
> +	 */
> +	if (off == 0 && !suspect)  {
> +		for (j = 0;
> +		     j < XFS_INODES_PER_CHUNK;
> +		     j += mp->m_sb.sb_inopblock)  {
> +			agbno = XFS_AGINO_TO_AGBNO(mp, ino + j);
> +			state = get_agbno_state(mp, agno, agbno);
> +			if (state == XR_E_UNKNOWN)  {
> +				set_agbno_state(mp, agno, agbno, XR_E_INO);
> +			} else if (state == XR_E_INUSE_FS && agno == 0 &&
> +				   ino + j >= first_prealloc_ino &&
> +				   ino + j < last_prealloc_ino)  {
> +				set_agbno_state(mp, agno, agbno, XR_E_INO);
> +			} else  {
> +				do_warn(
> +_("inode chunk claims used block, inobt block - agno %d, bno %d, inopb %d\n"),
> +					agno, agbno,
> +				mp->m_sb.sb_inopblock);

pretty weird indentation here can't you just merge w/ previous line?


Also is the change from bno to agbno intentional in the message?
I guess it's fine.

...


> +		for (i = 0; i < numrecs; i++)
> +			suspect = scan_single_ino_chunk(agno, &rp[i], suspect);
>  
>  		if (suspect)
>  			bad_ino_btree = 1;
It seems like it might be nicer to just do:


+		for (i = 0; i < numrecs; i++)
+			suspect += scan_single_ino_chunk(agno, &rp[i]);

and let scan_single_ino_chunk return 0/1 instead of passing suspect in
and returning an incremented value?

Hm but I guess the sub-function tests it doesn't it:

+       /*
+        * set state of each block containing inodes
+        */
+       if (off == 0 && !suspect)  {

so, seems fine as-is, though I'd just fix that indentation.

Thanks,
-Eric


-Eric

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