Re: [PATCH v2 1/3] btrfs: scrub: rename members related to scrub_block::pagev

David Sterba
2022-03-14T19:21:59Z

On Sat, Mar 12, 2022 at 07:17:42AM +0800, Qu Wenruo wrote:
> >> @@ -1078,16 +1078,16 @@ static int scrub_handle_errored_block(struct scrub_block *sblock_to_check)
> >> * area are unreadable.
> >> */
> >> success = 1;
> >> - for (page_num = 0; page_num < sblock_bad->page_count;
> >> - page_num++) {
> >> - struct scrub_page *spage_bad = sblock_bad->pagev[page_num];
> >> + for (sector_num = 0; sector_num < sblock_bad->sector_count;
> >
> > This is a simple indexing, so while sector_num is accurate a plain 'i'
> > would work too. It would also make some lines shorter.
>
> Here I intentionally avoid using single letter, because the existing
> code is doing a pretty bad practice by doing a double for loop.
>
> Here we're doing two different loops, one is iterating all the sectors,
> the other one is iterating all the mirrors.
>
> Thus we need to distinguish them, or it' can easily get screwed up using
> different loop indexes.

Yeah in this case it makes more sense to keep the descriptive name.