[prev in list] [next in list] [prev in thread] [next in thread]
List: busybox
Subject: AW: [PATCH 2/9] loop:refactor: extract subfunction get_next_free_loop()
From: Walter Harms <wharms () bfs ! de>
Date: 2022-11-18 12:20:04
Message-ID: b81b208478404dc294a0e74f66e86e33 () bfs ! de
[Download RAW message or body]
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 <busybox-bounces@busybox.net> im Auftrag von Xiaoming Ni <nixiaoming@huawei.com>
Gesendet: Freitag, 18. November 2022 02:01:51
An: busybox@busybox.net; vda.linux@googlemail.com
Cc: wangle6@huawei.com
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 <nixiaoming@huawei.com>
---
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
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox
_______________________________________________
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic