hi, just a minor comment. do not use i as name for return value, most ppl use it as loop counter, triggers the wrong circuits in the brain. (rule of least surprise). just name it err or what you like,
and please untangle the if() if (err>=0) return if (err==-2) return return -1 just my 2 cents ... ________________________________________ Von: busybox <[email protected]> im Auftrag von Xiaoming Ni <[email protected]> Gesendet: Freitag, 18. November 2022 02:01:51 An: [email protected]; [email protected] Cc: [email protected] Betreff: [PATCH 2/9] loop:refactor: extract subfunction get_next_free_loop() Step 2 of micro-refactoring the set_loop function () Extract subfunction get_next_free_loop() from set_loop() Also fix miss free(try) when stat(try) and mknod fail function old new delta set_loop 758 734 -24 ------------------------------------------------------------------------------ (add/remove: 0/0 grow/shrink: 0/1 up/down: 0/-24) Total: -24 bytes Fixes: 3448914e8cc5 ("mount,losetup: use /dev/loop-control is it exists") Signed-off-by: Xiaoming Ni <[email protected]> --- libbb/loop.c | 55 ++++++++++++++++++++++++---------------------------- 1 file changed, 25 insertions(+), 30 deletions(-) diff --git a/libbb/loop.c b/libbb/loop.c index c517ceb13..71fd8c1bc 100644 --- a/libbb/loop.c +++ b/libbb/loop.c @@ -96,6 +96,20 @@ int FAST_FUNC get_free_loop(void) return loopdevno; /* can be -1 if error */ } +static int get_next_free_loop(char *dev, int id) +{ + int i = get_free_loop(); + if (i >= 0) { + sprintf(dev, LOOP_FORMAT, i); + return 1; /* use /dev/loop-control */ + } else if (i == -2) { + sprintf(dev, LOOP_FORMAT, id); + return 2; + } else { + return -1; /* no free loop devices */ + } +} + static int open_file(const char *file, unsigned flags, int *mode) { int ffd; @@ -132,30 +146,26 @@ int FAST_FUNC set_loop(char **device, const char *file, unsigned long long offse try = *device; if (!try) { - get_free_loopN: - i = get_free_loop(); - if (i == -1) { - close(ffd); - return -1; /* no free loop devices */ - } - if (i >= 0) { - try = xasprintf(LOOP_FORMAT, i); - goto open_lfd; - } - /* i == -2: no /dev/loop-control. Do an old-style search for a free device */ try = dev; } /* Find a loop device */ /* 0xfffff is a max possible minor number in Linux circa 2010 */ for (i = 0; i <= 0xfffff; i++) { - sprintf(dev, LOOP_FORMAT, i); + if (!*device) { + rc = get_next_free_loop(dev, i); + if (rc == -1) { + break; /* no free loop devices */ + } else if (rc == 1) { + goto open_lfd; + } + } IF_FEATURE_MOUNT_LOOP_CREATE(errno = 0;) if (stat(try, &statbuf) != 0 || !S_ISBLK(statbuf.st_mode)) { if (ENABLE_FEATURE_MOUNT_LOOP_CREATE && errno == ENOENT - && try == dev + && (!*device) ) { /* Node doesn't exist, try to create it */ if (mknod(dev, S_IFBLK|0644, makedev(7, i)) == 0) @@ -188,13 +198,10 @@ int FAST_FUNC set_loop(char **device, const char *file, unsigned long long offse /* Associate free loop device with file */ if (ioctl(lfd, LOOP_SET_FD, ffd)) { /* Ouch. Are we racing with other mount? */ - if (!*device /* yes */ - && try != dev /* tried a _kernel-offered_ loopN? */ - ) { - free(try); + if (!*device) { close(lfd); //TODO: add "if (--failcount != 0) ..."? - goto get_free_loopN; + continue; } goto close_and_try_next_loopN; } @@ -218,8 +225,6 @@ int FAST_FUNC set_loop(char **device, const char *file, unsigned long long offse } if (rc == 0) { /* SUCCESS! */ - if (try != dev) /* tried a kernel-offered free loopN? */ - *device = try; /* malloced */ if (!*device) /* was looping in search of free "/dev/loopN"? */ *device = xstrdup(dev); rc = lfd; /* return this */ @@ -227,16 +232,6 @@ int FAST_FUNC set_loop(char **device, const char *file, unsigned long long offse } /* failure, undo LOOP_SET_FD */ ioctl(lfd, LOOP_CLR_FD, 0); // actually, 0 param is unnecessary - } else { - /* device is not free (rc == 0), or error other than ENXIO */ - if (rc == 0 /* device is not free? */ - && !*device /* racing with other mount? */ - && try != dev /* tried a _kernel-offered_ loopN? */ - ) { - free(try); - close(lfd); - goto get_free_loopN; - } } close_and_try_next_loopN: close(lfd); -- 2.27.0 _______________________________________________ busybox mailing list [email protected] http://lists.busybox.net/mailman/listinfo/busybox _______________________________________________ busybox mailing list [email protected] http://lists.busybox.net/mailman/listinfo/busybox
