On 2/5/12 5:44 PM, Michael Tokarev wrote: > On 05.02.2012 00:10, Eric Sandeen wrote: > [] > > Just a very quick look: > >> * sparsify - utility to punch out blocks of 0s in a file >> int main(int argc, char **argv) >> { > [] >> if (optind == argc) { >> printf("Error: no filename specified\n"); >> usage(); >> } >> >> fname = argv[optind++]; > > There's no handling of the case when there are more than one file > specified on the command line. ok > >> /* >> * Normalize to blocksize-aligned range: >> * round start down, round end up - get all blocks including the range specified >> */ >> >> punch_range_start = round_down(punch_range_start, blocksize); >> punch_range_end = round_up(punch_range_end, blocksize); >> min_hole = round_up(min_hole, blocksize); >> if (!min_hole) >> min_hole = blocksize; > > I think this deserves some bold warning if punch_range_start > or punch_hole_end is not a multiple of blocksize. well, we can only punch on block boundaries. But I suppose I should swap round_up and round_down, so that we never punch anything that isn't *inside* the specified range. > [] >> /* >> * Read through the file, finding block-aligned regions of 0s. >> * If the region is at least min_hole, punch it out. >> * This should be starting at a block-aligned offset >> */ >> >> while ((ret = read(fd, readbuf, min_hole)) > 0) { >> >> if (!memcmp(readbuf, zerobuf, min_hole)) { > > Now this is interesting. Can ret be < min_hole? Can a read > in a middle of a file be shorter than specified? yes, and yes (but unlikely i think)... > How it will work together with some other operation being done > at the same file -- ftruncate anyone? I probably have some boundary condition & error checking to do yet :) Thanks for the review, -Eric > Thanks! > > /mjt -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html