From: Karel Zak Date: Thu, 11 Dec 2025 12:39:40 +0000 (+0100) Subject: fallocate: refactor --report-holes and --dig-holes output X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=3d214e7b1ced645b7092bb1aa999fead029fd312;p=thirdparty%2Futil-linux.git fallocate: refactor --report-holes and --dig-holes output This commit consolidates duplicate printf() calls and calculations by introducing two new helper functions: - update_holestat(): Tracks hole statistics and prints verbose output for both HOLETYPE_FILE and HOLETYPE_DATA. Statistics are always collected regardless of report mode. - summary_holestat(): Prints summary output for file holes, data holes, and "converted to sparse holes" message. Uses consistent formatting with hole counts and percentages. Changes: - Made 'report' a global variable (like 'verbose') - Renamed struct holes_report to struct holestat for consistency - Changed struct fields to uintmax_t to eliminate casts in printf - Removed 'report' parameter from dig_holes() function - Unified dig and report modes to use single dig_holes() call - All output strings wrapped in _() for translation - Consistent printf() usage instead of mixed fprintf() - Percentage format: "(%ju bytes, %.2f%% of the file)" The refactoring eliminates ~40 lines of duplicate code while maintaining all functionality and improving code maintainability. Signed-off-by: Karel Zak --- diff --git a/sys-utils/fallocate.c b/sys-utils/fallocate.c index 5404a9558..c71dda214 100644 --- a/sys-utils/fallocate.c +++ b/sys-utils/fallocate.c @@ -73,6 +73,7 @@ #include "optutils.h" static int verbose; +static int report; static char *filename; static void __attribute__((__noreturn__)) usage(void) @@ -170,17 +171,88 @@ static int is_nul(void *buf, size_t bufsize) return cbuf + bufsize < cp; } -static void dig_holes(int fd, off_t file_off, off_t len, bool report) +struct holestat { + uintmax_t fileholes_sz; + uintmax_t nfileholes; + + uintmax_t dataholes_sz; + uintmax_t ndataholes; + + uintmax_t filesz; +}; + +enum { + HOLETYPE_FILE = 1, + HOLETYPE_DATA +}; + +static void update_holestat(struct holestat *hstat, + off_t start, off_t end, int type) +{ + uintmax_t size = end - start; + + switch(type) { + case HOLETYPE_FILE: + hstat->fileholes_sz += size; + hstat->nfileholes++; + if (report && verbose) + printf(_("file hole: offset %ju - %ju (%ju bytes)\n"), + (uintmax_t)start, (uintmax_t)end, size); + break; + case HOLETYPE_DATA: + hstat->dataholes_sz += size; + hstat->ndataholes++; + if (report && verbose) + printf(_("data hole: offset %ju - %ju (%ju bytes)\n"), + (uintmax_t)start, (uintmax_t)end, size); + break; + } +} + +static void summary_holestat(struct holestat *hstat, int type) +{ + char *str; + double pct = 0.0; + + switch(type) { + case HOLETYPE_FILE: + if (hstat->filesz) + pct = (double)hstat->fileholes_sz / (double)hstat->filesz * 100.0; + str = size_to_human_string(SIZE_SUFFIX_3LETTER | SIZE_SUFFIX_SPACE, hstat->fileholes_sz); + printf(_("file holes: %ju holes, %s (%ju bytes, %.2f%% of the file)\n"), + hstat->nfileholes, str, hstat->fileholes_sz, pct); + free(str); + break; + case HOLETYPE_DATA: + if (hstat->filesz) + pct = (double)hstat->dataholes_sz / (double)hstat->filesz * 100.0; + str = size_to_human_string(SIZE_SUFFIX_3LETTER | SIZE_SUFFIX_SPACE, hstat->dataholes_sz); + printf(_("data holes: %ju holes, %s (%ju bytes, %.2f%% of the file)\n"), + hstat->ndataholes, str, hstat->dataholes_sz, pct); + free(str); + break; + default: + /* dig mode: converted to sparse holes */ + if (hstat->filesz) + pct = (double)hstat->dataholes_sz / (double)hstat->filesz * 100.0; + str = size_to_human_string(SIZE_SUFFIX_3LETTER | SIZE_SUFFIX_SPACE, hstat->dataholes_sz); + printf(_("%s: %ju holes, %s (%ju bytes, %.2f%% of the file) converted to sparse holes.\n"), + filename, hstat->ndataholes, str, hstat->dataholes_sz, pct); + free(str); + break; + } +} + +static void dig_holes(int fd, off_t file_off, off_t len) { off_t file_end = len ? file_off + len : 0; off_t hole_start = 0, hole_sz = 0; - uintmax_t ct = 0; - uintmax_t total_file_hole_sz = 0; - off_t file_size = 0; off_t last_end = 0; size_t bufsz; char *buf; struct stat st; + struct holestat hstat = { .filesz = 0 }; + #if defined(POSIX_FADV_SEQUENTIAL) && defined(HAVE_POSIX_FADVISE) off_t cache_start = file_off; /* @@ -198,7 +270,7 @@ static void dig_holes(int fd, off_t file_off, off_t len, bool report) err(EXIT_FAILURE, _("stat of %s failed"), filename); bufsz = st.st_blksize; - file_size = st.st_size; + hstat.filesz = st.st_size; if (lseek(fd, file_off, SEEK_SET) < 0) err(EXIT_FAILURE, _("seek on %s failed"), filename); @@ -217,24 +289,12 @@ static void dig_holes(int fd, off_t file_off, off_t len, bool report) break; end = lseek(fd, off, SEEK_HOLE); - if (report && end > off && last_end < off) { - /* file hole position found from last_end to off */ - off_t hole_size = off - last_end; - total_file_hole_sz += hole_size; - if (verbose) - printf("file hole: offset %ju - %ju: (%ju bytes)\n", - (uintmax_t)last_end, (uintmax_t)off, (uintmax_t)hole_size); - } else if (report && off > end) { - /* file hole position found from end to off */ - off_t hole_size = off - end; - total_file_hole_sz += hole_size; - if (verbose) - printf("file hole: offset %ju - %ju: (%ju bytes)\n", - (uintmax_t)end, (uintmax_t)off, (uintmax_t)hole_size); - } - if (report) { - last_end = end; - } + if (end > off && last_end < off) + update_holestat(&hstat, last_end, off, HOLETYPE_FILE); + else if (off > end) + update_holestat(&hstat, end, off, HOLETYPE_FILE); + + last_end = end; if (file_end && end > file_end) end = file_end; @@ -264,11 +324,7 @@ static void dig_holes(int fd, off_t file_off, off_t len, bool report) if (!report) xfallocate(fd, FALLOC_FL_PUNCH_HOLE|FALLOC_FL_KEEP_SIZE, hole_start, hole_sz); - if (report && verbose) - printf("data hole: offset %ju - %ju: (%ju bytes)\n", - (uintmax_t)hole_start, (uintmax_t)(hole_start+hole_sz), - (uintmax_t)hole_sz); - ct += hole_sz; + update_holestat(&hstat, hole_start, hole_start + hole_sz, HOLETYPE_DATA); hole_sz = hole_start = 0; } @@ -285,46 +341,26 @@ static void dig_holes(int fd, off_t file_off, off_t len, bool report) off += rsz; } if (hole_sz) { - if (report && verbose) - printf("data hole: offset %ju - %ju: (%ju bytes)\n", - (uintmax_t)hole_start, (uintmax_t)(hole_start+hole_sz), - (uintmax_t)hole_sz); off_t alloc_sz = hole_sz; if (off >= end) alloc_sz += st.st_blksize; /* meet block boundary */ if (!report) xfallocate(fd, FALLOC_FL_PUNCH_HOLE|FALLOC_FL_KEEP_SIZE, hole_start, alloc_sz); - ct += hole_sz; - if(report) - hole_sz = 0; + update_holestat(&hstat, hole_start, hole_start + hole_sz, HOLETYPE_DATA); + hole_sz = 0; } file_off = off; } free(buf); - if (!report && verbose) { - char *str = size_to_human_string(SIZE_SUFFIX_3LETTER | SIZE_SUFFIX_SPACE, ct); - fprintf(stdout, _("%s: %s (%ju bytes) converted to sparse holes.\n"), - filename, str, ct); - free(str); - } if (report) { - printf("\nSummary:\n"); - /* file holes summary */ - double file_pct = (file_size == 0) ? 0.0 : (double)total_file_hole_sz / (double)file_size * 100.0; - char *str = size_to_human_string(SIZE_SUFFIX_3LETTER | SIZE_SUFFIX_SPACE, total_file_hole_sz); - printf("file holes: %s (%ju bytes) %.2f%% of the file\n", - str, total_file_hole_sz, file_pct); - free(str); - /* data holes summary */ - char *str1 = size_to_human_string(SIZE_SUFFIX_3LETTER | SIZE_SUFFIX_SPACE, ct); - double data_pct = (file_size == 0) ? 0.0 : (double)ct / (double)file_size * 100.0; - printf("data holes: %s (%ju bytes) %.2f%% of the file\n", - str1, ct, data_pct); - free(str1); - } + printf(_("\nSummary:\n")); + summary_holestat(&hstat, HOLETYPE_FILE); + summary_holestat(&hstat, HOLETYPE_DATA); + } else if (verbose) + summary_holestat(&hstat, 0); } int main(int argc, char **argv) @@ -333,7 +369,6 @@ int main(int argc, char **argv) int fd; int mode = 0; int dig = 0; - int report = 0; int posix = 0; loff_t length = -2LL; loff_t offset = 0; @@ -428,13 +463,13 @@ int main(int argc, char **argv) if (optind != argc) errx(EXIT_FAILURE, _("unexpected number of arguments")); - if (dig) { - /* for --dig-holes the default is analyze all file */ + if (dig || report) { + /* for --dig-holes and --report-holes the default is analyze all file */ if (length == -2LL) length = 0; if (length < 0) errx(EXIT_FAILURE, _("invalid length")); - } else if (!report) { + } else { /* it's safer to require the range specification (--length --offset) */ if (length == -2LL) errx(EXIT_FAILURE, _("no length argument specified")); @@ -451,10 +486,8 @@ int main(int argc, char **argv) if (fd < 0) err(EXIT_FAILURE, _("cannot open %s"), filename); - if (dig) - dig_holes(fd, offset, length, false); - else if (report) - dig_holes(fd, offset, 0, true); + if (dig || report) + dig_holes(fd, offset, length); else { if (posix) xposix_fallocate(fd, offset, length);