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

Eric Sandeen
2009-10-12T20:05:13Z

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