On 2/10/16 9:43 AM, Bill O'Donnell wrote: >> I'd make __find_secondary_sb() take (sb, start, skip) i.e. send in >> > this: >> > >>> > > + retval = __find_secondary_sb(rsb, agsize, agsize); >>> > > + if (!retval) { >>> > > + /* >>> > > + * fallback: use minimum agsize for skipsize >>> > > + */ >>> > > + retval = __find_secondary_sb(rsb, XFS_AG_MIN_BYTES, BSIZE); >>> > > + } >> > >> > and the function is something like: >> > >> > static int >> > __find_secondary_sb( >> > xfs_sb_t *rsb, >> > xfs_off_t start, >> > xfs_off_t skip) >> > >> > { >> > >> > ... >> > >> > for (done = 0, off = start; !done ; off += skip) { >> > ... >> > if (lseek64(x.dfd, off, SEEK_SET) != off) >> > done = 1; >> > >> > if (!done && (read(x.dfd, sb, BSIZE)) <= 0) >> > done = 1; > But, bsize is used here: > ... > * check the buffer 512 bytes at a time since > * we don't know how big the sectors really are. > */ > for (i = 0; !done && i < bsize; i += BBSIZE) { > ... > so, don't we still need to populate bsize? Or does it make more > sense to just use BBSIZE in the conditional, ala: > for (i = 0; !done && i < BBSIZE; i += BBSIZE) Oh, right, sorry. yes, keep the assignment: if (!done && (bsize = read(x.dfd, sb, BSIZE)) <= 0) { That handles a short read at the end; we ask for BSIZE, actually read bsize, and then we need to iterate over what actually got read (bsize). BSIZE, bsize ... clear as mud. :( -Eric >> > >> > >> > because you really can't deduce both the starting point and the skip-ahead >> > size from just one parameter. > Agreed. > Thanks for your thorough reviews :) > Bill > _______________________________________________ xfs mailing list xfs@xxxxxxxxxxx http://oss.sgi.com/mailman/listinfo/xfs