On 2022/11/18 20:36, Walter Harms wrote:
A little tweak make it more readable (i hope)
putting if (rc) direkt behind

   rc = ioctl(lfd, BB_LOOP_SET_STATUS, &loopinfo);
   if (rc == 0)
           return lfd;

// no need to check rc again
if ( (loopinfo.lo_flags & BB_LO_FLAGS_AUTOCLEAR)) {
// try again without   BB_LO_FLAGS_AUTOCLEAR
// does it realy work that way ? no ~ ?
loopinfo.lo_flags -= BB_LO_FLAGS_AUTOCLEAR;
   rc = ioctl(lfd, BB_LOOP_SET_STATUS, &loopinfo);
}
  if (rc == 0)
              return lfd;

+       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;
+       }

In this case, duplicate "if(rc==0)" occurs.
Is that worse?

Thanks
Xiaoming Ni


________________________________________
Von: busybox <busybox-boun...@busybox.net> im Auftrag von Xiaoming Ni 
<nixiaom...@huawei.com>
Gesendet: Freitag, 18. November 2022 13:14:44
An: busybox@busybox.net; vda.li...@googlemail.com; c...@gmx.com; 
explore...@gmail.com
Cc: wang...@huawei.com
Betreff: [PATCH v2 5/9] loop:refactor: extract subfunction set_loop_configure()

Step 5 of micro-refactoring the set_loop():
         Extract subfunction set_loop_configure()

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

Signed-off-by: Xiaoming Ni <nixiaom...@huawei.com>
---
  libbb/loop.c | 63 ++++++++++++++++++++++++++++++----------------------
  1 file changed, 36 insertions(+), 27 deletions(-)

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

+static int set_loop_configure(int ffd, int lfd, const char *file,
+               unsigned long long offset, unsigned long long sizelimit, 
unsigned flags)
+{
+       int rc;
+       bb_loop_info loopinfo;
+       /* 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;
+}
+
  static int set_loop_info(int ffd, int lfd, const char *file,
                 unsigned long long offset, unsigned long long sizelimit, 
unsigned flags)
  {
@@ -136,33 +171,7 @@ static int set_loop_info(int ffd, int lfd, const char 
*file,

         /* 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 set_loop_configure(ffd, lfd, file, offset, sizelimit, 
flags);
         }
         return -1;
  }
--
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

Reply via email to