shared/loop-util: fix error handling in loop_device_make_full()

The function no longer returns the fd. This complicated semantics, because it
wasn't clear what holds the ownership: the return value or the output
parameter.  There were no users of the fd in the return value, so let's
simplify things conceptually and only return the fd once.

Reduce the scope of variables.

LOOP_CLR_FD was called on the wrong fd. Let's use a cleanup function to make
this automatic and reduce chances of a mixup in the future.

CID 1408498.
This commit is contained in:
Zbigniew Jędrzejewski-Szmek 2019-12-06 11:10:10 +01:00
parent 6b2a8b80b4
commit e8af3bfd63
1 changed files with 26 additions and 31 deletions

View File

@ -20,6 +20,13 @@
#include "stat-util.h"
#include "stdio-util.h"
static void cleanup_clear_loop_close(int *fd) {
if (*fd >= 0) {
(void) ioctl(*fd, LOOP_CLR_FD);
(void) safe_close(*fd);
}
}
int loop_device_make_full(
int fd,
int open_flags,
@ -28,9 +35,7 @@ int loop_device_make_full(
uint32_t loop_flags,
LoopDevice **ret) {
_cleanup_close_ int control = -1, loop = -1;
_cleanup_free_ char *loopdev = NULL;
unsigned n_attempts = 0;
struct loop_info64 info;
LoopDevice *d = NULL;
struct stat st;
@ -69,7 +74,6 @@ int loop_device_make_full(
d = new(LoopDevice, 1);
if (!d)
return -ENOMEM;
*d = (LoopDevice) {
.fd = TAKE_FD(copy),
.nr = nr,
@ -86,13 +90,18 @@ int loop_device_make_full(
return r;
}
_cleanup_close_ int control = -1;
_cleanup_(cleanup_clear_loop_close) int loop_with_fd = -1;
control = open("/dev/loop-control", O_RDWR|O_CLOEXEC|O_NOCTTY|O_NONBLOCK);
if (control < 0)
return -errno;
/* Loop around LOOP_CTL_GET_FREE, since at the moment we attempt to open the returned device it might
* be gone already, taken by somebody else racing against us. */
for (;;) {
for (unsigned n_attempts = 0;;) {
_cleanup_close_ int loop = -1;
nr = ioctl(control, LOOP_CTL_GET_FREE);
if (nr < 0)
return -errno;
@ -103,17 +112,16 @@ int loop_device_make_full(
loop = open(loopdev, O_CLOEXEC|O_NONBLOCK|O_NOCTTY|open_flags);
if (loop < 0)
return -errno;
if (ioctl(loop, LOOP_SET_FD, fd) < 0) {
if (errno != EBUSY)
return -errno;
if (++n_attempts >= 64) /* Give up eventually */
return -EBUSY;
} else
if (ioctl(loop, LOOP_SET_FD, fd) >= 0) {
loop_with_fd = TAKE_FD(loop);
break;
}
if (errno != EBUSY)
return -errno;
if (++n_attempts >= 64) /* Give up eventually */
return -EBUSY;
loopdev = mfree(loopdev);
loop = safe_close(loop);
}
info = (struct loop_info64) {
@ -123,33 +131,20 @@ int loop_device_make_full(
.lo_sizelimit = size == UINT64_MAX ? 0 : size,
};
if (ioctl(loop, LOOP_SET_STATUS64, &info) < 0) {
r = -errno;
goto fail;
}
if (ioctl(loop_with_fd, LOOP_SET_STATUS64, &info) < 0)
return -errno;
d = new(LoopDevice, 1);
if (!d) {
r = -ENOMEM;
goto fail;
}
if (!d)
return -ENOMEM;
*d = (LoopDevice) {
.fd = TAKE_FD(loop),
.fd = TAKE_FD(loop_with_fd),
.node = TAKE_PTR(loopdev),
.nr = nr,
};
*ret = d;
return d->fd;
fail:
if (fd >= 0)
(void) ioctl(fd, LOOP_CLR_FD);
if (d && d->fd >= 0)
(void) ioctl(d->fd, LOOP_CLR_FD);
return r;
return 0;
}
int loop_device_make_by_path(const char *path, int open_flags, uint32_t loop_flags, LoopDevice **ret) {