Applied with some changes, thank you
On Mon, Nov 21, 2022 at 2:58 PM Xiaoming Ni <[email protected]> wrote: > > 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 | 58 +++++++++++++++++++++++++--------------------------- > 1 file changed, 28 insertions(+), 30 deletions(-) > > diff --git a/libbb/loop.c b/libbb/loop.c > index c517ceb13..6c28e683a 100644 > --- a/libbb/loop.c > +++ b/libbb/loop.c > @@ -9,6 +9,7 @@ > */ > #include "libbb.h" > #include <linux/version.h> > +#include <assert.h> > > #if LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,0) > > @@ -96,6 +97,22 @@ int FAST_FUNC get_free_loop(void) > return loopdevno; /* can be -1 if error */ > } > > +static int get_next_free_loop(char *dev, __attribute__((unused)) size_t > dev_size, int id) > +{ > + int loopdevno; > + assert(dev_size >= LOOP_NAMESIZE); > + loopdevno = get_free_loop(); > + if (loopdevno >= 0) { > + sprintf(dev, LOOP_FORMAT, loopdevno); > + return 1; /* use /dev/loop-control */ > + } > + if (loopdevno == -2) { > + sprintf(dev, LOOP_FORMAT, id); > + return 2; > + } > + return -1; /* no free loop devices */ > +} > + > static int open_file(const char *file, unsigned flags, int *mode) > { > int ffd; > @@ -132,30 +149,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, sizeof(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 +201,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 +228,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 +235,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
