On Friday, November 18, 2022, Xiaoming Ni <[email protected]> wrote:
> Step 6 of micro-refactoring the set_loop():
> Use structs to avoid transferring a large number of parameters
> in set_loop_configure() and set_loop_info()
>
> function old new delta
> set_loop 708 720 +12
>
------------------------------------------------------------------------------
> (add/remove: 0/0 grow/shrink: 1/0 up/down: 12/0) Total: 12
bytes
>
> Signed-off-by: Xiaoming Ni <[email protected]>
> ---
> libbb/loop.c | 54 +++++++++++++++++++++++++++++-----------------------
> 1 file changed, 30 insertions(+), 24 deletions(-)
>
> diff --git a/libbb/loop.c b/libbb/loop.c
> index 914af57b2..2200ccb9a 100644
> --- a/libbb/loop.c
> +++ b/libbb/loop.c
> @@ -126,32 +126,21 @@ 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)
> +static int set_loop_configure(int ffd, int lfd, bb_loop_info *loopinfo)
> {
> int rc;
> - bb_loop_info loopinfo;
> + bb_loop_info loopinfo2;
> /* 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)) {
> + rc = ioctl(lfd, BB_LOOP_SET_STATUS, loopinfo);
> + if (rc != 0 && (loopinfo->lo_flags & BB_LO_FLAGS_AUTOCLEAR)) {
> + memcpy(&loopinfo2, loopinfo, sizeof(*loopinfo));
Just use `loopinfo2 = loopinfo;`
Also, it is unclear why there is the need to clone the loopinfo buffer.
> /* 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);
> + loopinfo2.lo_flags -= BB_LO_FLAGS_AUTOCLEAR;
> + rc = ioctl(lfd, BB_LOOP_SET_STATUS, &loopinfo2);
> }
> if (rc == 0) {
> return lfd;
> @@ -161,21 +150,36 @@ static int set_loop_configure(int ffd, int lfd,
const char *file,
> return -1;
> }
>
> -static int set_loop_info(int ffd, int lfd, const char *file,
> - unsigned long long offset, unsigned long long sizelimit,
unsigned flags)
> +static int set_loop_info(int ffd, int lfd, bb_loop_info *loopinfo)
> {
> int rc;
> - bb_loop_info loopinfo;
> + bb_loop_info tmp;
>
> - rc = ioctl(lfd, BB_LOOP_GET_STATUS, &loopinfo);
> + rc = ioctl(lfd, BB_LOOP_GET_STATUS, &tmp);
>
> /* If device is free, try to claim it */
> if (rc && errno == ENXIO) {
> - return set_loop_configure(ffd, lfd, file, offset,
sizelimit, flags);
> + return set_loop_configure(ffd, lfd, loopinfo);
> }
> return -1;
> }
>
> +static void init_bb_loop_info(bb_loop_info *loopinfo, const char *file,
> + unsigned long long offset, unsigned long long sizelimit,
unsigned flags)
> +{
> + memset(loopinfo, 0, sizeof(*loopinfo));
Would it reduce code size by doing the initialization like this?
```
bb_loop_info empty = {0};
loopinfo = empty;
```
> + 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);
> +}
> +
> /* 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.
> @@ -187,12 +191,14 @@ int FAST_FUNC set_loop(char **device, const char
*file, unsigned long long offse
> char *try;
> struct stat statbuf;
> int i, lfd, ffd, mode, rc;
> + bb_loop_info loopinfo;
>
> ffd = open_file(file, flags, &mode);
> if (ffd < 0) {
> return -errno;
> }
>
> + init_bb_loop_info(&loopinfo, file, offset, sizelimit, flags);
> try = *device;
> if (!try) {
> try = dev;
> @@ -239,7 +245,7 @@ int FAST_FUNC set_loop(char **device, const char
*file, unsigned long long offse
> }
> goto try_next_loopN;
> }
> - rc = set_loop_info(ffd, lfd, file, offset, sizelimit,
flags);
> + rc = set_loop_info(ffd, lfd, &loopinfo);
> if (rc == lfd) {
> /* SUCCESS! */
> if (!*device)
> --
> 2.27.0
>
>
_______________________________________________
busybox mailing list
[email protected]
http://lists.busybox.net/mailman/listinfo/busybox
