AW: [PATCH v2 4/9] loop:refactor: extract subfunction set_loop_info()

Walter Harms wharms at bfs.de
Fri Nov 18 12:27:30 UTC 2022


on other minor

 if (rc && errno == ENXIO)

turn it on its head safes one indent level

if ( !rc || errno != ENXIO )
   return -1;   // failed to get loopinfo

jm2c
________________________________________
Von: busybox <busybox-bounces at busybox.net> im Auftrag von Xiaoming Ni <nixiaoming at huawei.com>
Gesendet: Freitag, 18. November 2022 13:14:43
An: busybox at busybox.net; vda.linux at googlemail.com; cand at gmx.com; explorer09 at gmail.com
Cc: wangle6 at huawei.com
Betreff: [PATCH v2 4/9] loop:refactor: extract subfunction set_loop_info()

Step 4 of micro-refactoring the set_loop():
        Extract subfunction set_loop_info() from set_loop()

function                                             old     new   delta
set_loop                                             734     700     -34
------------------------------------------------------------------------------
(add/remove: 0/0 grow/shrink: 0/1 up/down: 0/-34)             Total: -34 bytes

Signed-off-by: Xiaoming Ni <nixiaoming at huawei.com>
---
 libbb/loop.c | 91 +++++++++++++++++++++++++++-------------------------
 1 file changed, 47 insertions(+), 44 deletions(-)

diff --git a/libbb/loop.c b/libbb/loop.c
index 91c3a45c4..89adc4f94 100644
--- a/libbb/loop.c
+++ b/libbb/loop.c
@@ -126,6 +126,47 @@ static int open_file(const char *file, unsigned flags, int *mode)
        return ffd;
 }

+static int set_loop_info(int ffd, int lfd, const char *file,
+               unsigned long long offset, unsigned long long sizelimit, unsigned flags)
+{
+       int rc;
+       bb_loop_info loopinfo;
+
+       rc = ioctl(lfd, BB_LOOP_GET_STATUS, &loopinfo);
+
+       /* If device is free, try to claim it */
+       if (rc && errno == ENXIO) {
+               /* Associate free loop device with file */
+               if (ioctl(lfd, LOOP_SET_FD, ffd)) {
+                       return -1;
+               }
+               memset(&loopinfo, 0, sizeof(loopinfo));
+               safe_strncpy((char *)loopinfo.lo_file_name, file, LO_NAME_SIZE);
+               loopinfo.lo_offset = offset;
+               loopinfo.lo_sizelimit = sizelimit;
+               /*
+                * Used by mount to set LO_FLAGS_AUTOCLEAR.
+                * LO_FLAGS_READ_ONLY is not set because RO is controlled by open type of the file.
+                * Note that closing LO_FLAGS_AUTOCLEARed lfd before mount
+                * is wrong (would free the loop device!)
+                */
+               loopinfo.lo_flags = (flags & ~BB_LO_FLAGS_READ_ONLY);
+               rc = ioctl(lfd, BB_LOOP_SET_STATUS, &loopinfo);
+               if (rc != 0 && (loopinfo.lo_flags & BB_LO_FLAGS_AUTOCLEAR)) {
+                       /* Old kernel, does not support LO_FLAGS_AUTOCLEAR? */
+                       /* (this code path is not tested) */
+                       loopinfo.lo_flags -= BB_LO_FLAGS_AUTOCLEAR;
+                       rc = ioctl(lfd, BB_LOOP_SET_STATUS, &loopinfo);
+               }
+               if (rc == 0) {
+                       return lfd;
+               }
+               /* failure, undo LOOP_SET_FD */
+               ioctl(lfd, LOOP_CLR_FD, 0); // actually, 0 param is unnecessary
+       }
+       return -1;
+}
+
 /* Returns opened fd to the loop device, <0 on error.
  * *device is loop device to use, or if *device==NULL finds a loop device to
  * mount it on and sets *device to a strdup of that loop device name.
@@ -135,7 +176,6 @@ int FAST_FUNC set_loop(char **device, const char *file, unsigned long long offse
 {
        char dev[LOOP_NAMESIZE];
        char *try;
-       bb_loop_info loopinfo;
        struct stat statbuf;
        int i, lfd, ffd, mode, rc;

@@ -190,49 +230,12 @@ int FAST_FUNC set_loop(char **device, const char *file, unsigned long long offse
                        }
                        goto try_next_loopN;
                }
-
-               rc = ioctl(lfd, BB_LOOP_GET_STATUS, &loopinfo);
-
-               /* If device is free, try to claim it */
-               if (rc && errno == ENXIO) {
-                       /* Associate free loop device with file */
-                       if (ioctl(lfd, LOOP_SET_FD, ffd)) {
-                               close(lfd);
-                               /* Ouch. Are we racing with other mount? */
-                               if (!*device) {
-//TODO: add "if (--failcount != 0) ..."?
-                                       continue;
-                               } else {
-                                       break;
-                               }
-                       }
-                       memset(&loopinfo, 0, sizeof(loopinfo));
-                       safe_strncpy((char *)loopinfo.lo_file_name, file, LO_NAME_SIZE);
-                       loopinfo.lo_offset = offset;
-                       loopinfo.lo_sizelimit = sizelimit;
-                       /*
-                        * Used by mount to set LO_FLAGS_AUTOCLEAR.
-                        * LO_FLAGS_READ_ONLY is not set because RO is controlled by open type of the file.
-                        * Note that closing LO_FLAGS_AUTOCLEARed lfd before mount
-                        * is wrong (would free the loop device!)
-                        */
-                       loopinfo.lo_flags = (flags & ~BB_LO_FLAGS_READ_ONLY);
-                       rc = ioctl(lfd, BB_LOOP_SET_STATUS, &loopinfo);
-                       if (rc != 0 && (loopinfo.lo_flags & BB_LO_FLAGS_AUTOCLEAR)) {
-                               /* Old kernel, does not support LO_FLAGS_AUTOCLEAR? */
-                               /* (this code path is not tested) */
-                               loopinfo.lo_flags -= BB_LO_FLAGS_AUTOCLEAR;
-                               rc = ioctl(lfd, BB_LOOP_SET_STATUS, &loopinfo);
-                       }
-                       if (rc == 0) {
-                               /* SUCCESS! */
-                               if (!*device)   /* was looping in search of free "/dev/loopN"? */
-                                       *device = xstrdup(dev);
-                               rc = lfd; /* return this */
-                               break;
-                       }
-                       /* failure, undo LOOP_SET_FD */
-                       ioctl(lfd, LOOP_CLR_FD, 0); // actually, 0 param is unnecessary
+               rc = set_loop_info(ffd, lfd, file, offset, sizelimit, flags);
+               if (rc == lfd) {
+                       /* SUCCESS! */
+                       if (!*device)
+                               *device = xstrdup(dev);
+                       break;
                }
                close(lfd);
  try_next_loopN:
--
2.27.0

_______________________________________________
busybox mailing list
busybox at busybox.net
http://lists.busybox.net/mailman/listinfo/busybox


More information about the busybox mailing list