local: harden local_close() and use it for local_open() failure

There's a bug, when enabling a buffer (with local_buffer_enabled_set(),
i.e. writing to 'buffer/enable'), we can get -EINVAL, because the scan_mask
is invalidated by the kernel.
The issue seems to be that the local_open() exit-on-failure path is
un-balanced. Some things don't seem to be cleaned-up on exit-on-failure.

This is seen with iiod trying to open a high-speed buffer device, where the
exit-on-failure path does not call munmap() on the buffers, nor does it
free the memory allocated for the buffer (in libiio). On the next
retry/open the buffer device is locked and -EBUSY is returned.
Essentially, this leaks resources (a file-descriptor and some memory for
the buffers).
Closing and re-opening iiod fixes the issue.

Also, the local_close() function should not exit on the first fail in
finds. It should continue with disabling everything it can. The only error
that might make sense, is if the main FD is not initialized.

This change simplifies the local_close() a bit: we return -EBADF if the
main FD is -1. Other than that, we continue disabling/cleaning everything
else (with the proper NULL/fd checks in place).

This makes it re-usable for the local_open() exit-on-failure path.
This could be addressed [also] by balancing the initialization in the
reverse order of init. But it looks like something might be omitted later
(as they were up until now) in the exit-on-failure path.
So maybe a more future-proof method is to just re-use the local_open()
routine where possible.

Signed-off-by: Alexandru Ardelean <[email protected]>

local.c

@@ -901,6 +901,8 @@ static int enable_high_speed(const struct iio_device *dev)
 	return ret;
 }
 
+static int local_close(const struct iio_device *dev);
+
 static int local_open(const struct iio_device *dev,
 		size_t samples_count, bool cyclic)
 {
@@ -929,8 +931,8 @@ static int local_open(const struct iio_device *dev,
 	iio_snprintf(buf, sizeof(buf), "/dev/%s", dev->id);
 	pdata->fd = open(buf, O_RDWR | O_CLOEXEC | O_NONBLOCK);
 	if (pdata->fd == -1) {
-		ret = -errno;
-		goto err_close_cancel_fd;
+		close(pdata->cancel_fd);
+		return -errno;
 	}
 
 	/* Disable channels */
@@ -988,45 +990,42 @@ static int local_open(const struct iio_device *dev,
 
 	return 0;
 err_close:
-	close(pdata->fd);
-	pdata->fd = -1;
-err_close_cancel_fd:
-	close(pdata->cancel_fd);
-	pdata->cancel_fd = -1;
+	local_close(dev);
 	return ret;
 }
 
 static int local_close(const struct iio_device *dev)
 {
 	struct iio_device_pdata *pdata = dev->pdata;
 	unsigned int i;
-	int ret;
 
 	if (pdata->fd == -1)
 		return -EBADF;
 
 	if (pdata->is_high_speed) {
 		unsigned int i;
-		for (i = 0; i < pdata->allocated_nb_blocks; i++)
-			munmap(pdata->addrs[i], pdata->blocks[i].size);
-		ioctl_nointr(pdata->fd, BLOCK_FREE_IOCTL, 0);
+		if (pdata->addrs) {
+			for (i = 0; i < pdata->allocated_nb_blocks; i++)
+				munmap(pdata->addrs[i], pdata->blocks[i].size);
+		}
+		if (pdata->fd > -1)
+			ioctl_nointr(pdata->fd, BLOCK_FREE_IOCTL, 0);
 		pdata->allocated_nb_blocks = 0;
 		free(pdata->addrs);
 		pdata->addrs = NULL;
 		free(pdata->blocks);
 		pdata->blocks = NULL;
 	}
 
-	ret = close(pdata->fd);
-	if (ret)
-		return ret;
-
-	close(pdata->cancel_fd);
-
+	close(pdata->fd);
 	pdata->fd = -1;
-	pdata->cancel_fd = -1;
 
-	ret = local_buffer_enabled_set(dev, false);
+	if (pdata->cancel_fd > -1) {
+		close(pdata->cancel_fd);
+		pdata->cancel_fd = -1;
+	}
+
+	local_buffer_enabled_set(dev, false);
 
 	for (i = 0; i < dev->nb_channels; i++) {
 		struct iio_channel *chn = dev->channels[i];
@@ -1035,7 +1034,7 @@ static int local_close(const struct iio_device *dev)
 			channel_write_state(chn, false);
 	}
 
-	return (ret < 0) ? ret : 0;
+	return 0;
 }
 
 static int local_get_fd(const struct iio_device *dev)