diff mbox series

[2/4] loop: Factor out configuring loop from status.

Message ID 20200420080409.111693-3-maco@android.com (mailing list archive)
State New, archived
Headers show
Series Add a new LOOP_SET_FD_AND_STATUS ioctl. | expand

Commit Message

Martijn Coenen April 20, 2020, 8:04 a.m. UTC 
Factor out this code into a separate function, so it can be reused by
other code more easily.

Signed-off-by: Martijn Coenen <maco@android.com>
---
 drivers/block/loop.c | 108 +++++++++++++++++++++++--------------------
 1 file changed, 58 insertions(+), 50 deletions(-)

Comments

Bart Van Assche April 20, 2020, 1:34 p.m. UTC | #1 
On 4/20/20 1:04 AM, Martijn Coenen wrote:

No trailing dot at the end of a patch subject please.

>   static int
> -loop_set_status(struct loop_device *lo, const struct loop_info64 *info)
> +loop_set_from_status(struct loop_device *lo, const struct loop_info64 *info)
>   {
>   	int err;
>   	struct loop_func_table *xfer;
>   	kuid_t uid = current_uid();
> +
> +	if ((unsigned int) info->lo_encrypt_key_size > LO_KEY_SIZE)
> +		return -EINVAL;
> +
> +	err = loop_release_xfer(lo);
> +	if (err)
> +		return err;
> +
> +	if (info->lo_encrypt_type) {
> +		unsigned int type = info->lo_encrypt_type;
> +
> +		if (type >= MAX_LO_CRYPT)
> +			return -EINVAL;
> +		xfer = xfer_funcs[type];
> +		if (xfer == NULL)
> +			return -EINVAL;
> +	} else
> +		xfer = NULL;
> +
> +	err = loop_init_xfer(lo, xfer, info);
> +	if (err)
> +		return err;
> +
> +	lo->lo_offset = info->lo_offset;
> +	lo->lo_sizelimit = info->lo_sizelimit;
> +	memcpy(lo->lo_file_name, info->lo_file_name, LO_NAME_SIZE);
> +	memcpy(lo->lo_crypt_name, info->lo_crypt_name, LO_NAME_SIZE);
> +	lo->lo_file_name[LO_NAME_SIZE-1] = 0;
> +	lo->lo_crypt_name[LO_NAME_SIZE-1] = 0;
> +
> +	if (!xfer)
> +		xfer = &none_funcs;
> +	lo->transfer = xfer->transfer;
> +	lo->ioctl = xfer->ioctl;
> +
> +	if ((lo->lo_flags & LO_FLAGS_AUTOCLEAR) !=
> +	     (info->lo_flags & LO_FLAGS_AUTOCLEAR))
> +		lo->lo_flags ^= LO_FLAGS_AUTOCLEAR;
> +
> +	lo->lo_encrypt_key_size = info->lo_encrypt_key_size;
> +	lo->lo_init[0] = info->lo_init[0];
> +	lo->lo_init[1] = info->lo_init[1];
> +	if (info->lo_encrypt_key_size) {
> +		memcpy(lo->lo_encrypt_key, info->lo_encrypt_key,
> +		       info->lo_encrypt_key_size);
> +		lo->lo_key_owner = uid;
> +	}
> +
> +	return 0;
> +}

Please add a (one line?) comment above this function that explains the 
purpose of this function. Is the purpose of this function perhaps to 
initialize loop device parameters based on the information received from 
user space (the 'info' argument)?

Thanks,

Bart.
Bart Van Assche April 20, 2020, 1:49 p.m. UTC | #2 
On 4/20/20 1:04 AM, Martijn Coenen wrote:
>   static int
> -loop_set_status(struct loop_device *lo, const struct loop_info64 *info)
> +loop_set_from_status(struct loop_device *lo, const struct loop_info64 *info)
>   {

An additional question: since this function sets the status of 'lo' 
based on the information in 'info', would "loop_set_status" or 
"loop_set_status_from_info" be a better name for this function?

Thanks,

Bart.
Martijn Coenen April 21, 2020, 11:46 a.m. UTC | #3 
On Mon, Apr 20, 2020 at 3:49 PM Bart Van Assche <bvanassche@acm.org> wrote:
>
> An additional question: since this function sets the status of 'lo'
> based on the information in 'info', would "loop_set_status" or
> "loop_set_status_from_info" be a better name for this function?

Yeah, I like the latter. I will rename, and add a comment that
explains the purpose more clearly.

Thanks,
Martijn

>
> Thanks,
>
> Bart.
diff mbox series

Patch

diff --git a/drivers/block/loop.c b/drivers/block/loop.c
index d934d65dbe92..e0f9674fe568 100644
--- a/drivers/block/loop.c
+++ b/drivers/block/loop.c
@@ -1276,12 +1276,68 @@  static int loop_clr_fd(struct loop_device *lo)
 }
 
 static int
-loop_set_status(struct loop_device *lo, const struct loop_info64 *info)
+loop_set_from_status(struct loop_device *lo, const struct loop_info64 *info)
 {
 	int err;
 	struct loop_func_table *xfer;
 	kuid_t uid = current_uid();
+
+	if ((unsigned int) info->lo_encrypt_key_size > LO_KEY_SIZE)
+		return -EINVAL;
+
+	err = loop_release_xfer(lo);
+	if (err)
+		return err;
+
+	if (info->lo_encrypt_type) {
+		unsigned int type = info->lo_encrypt_type;
+
+		if (type >= MAX_LO_CRYPT)
+			return -EINVAL;
+		xfer = xfer_funcs[type];
+		if (xfer == NULL)
+			return -EINVAL;
+	} else
+		xfer = NULL;
+
+	err = loop_init_xfer(lo, xfer, info);
+	if (err)
+		return err;
+
+	lo->lo_offset = info->lo_offset;
+	lo->lo_sizelimit = info->lo_sizelimit;
+	memcpy(lo->lo_file_name, info->lo_file_name, LO_NAME_SIZE);
+	memcpy(lo->lo_crypt_name, info->lo_crypt_name, LO_NAME_SIZE);
+	lo->lo_file_name[LO_NAME_SIZE-1] = 0;
+	lo->lo_crypt_name[LO_NAME_SIZE-1] = 0;
+
+	if (!xfer)
+		xfer = &none_funcs;
+	lo->transfer = xfer->transfer;
+	lo->ioctl = xfer->ioctl;
+
+	if ((lo->lo_flags & LO_FLAGS_AUTOCLEAR) !=
+	     (info->lo_flags & LO_FLAGS_AUTOCLEAR))
+		lo->lo_flags ^= LO_FLAGS_AUTOCLEAR;
+
+	lo->lo_encrypt_key_size = info->lo_encrypt_key_size;
+	lo->lo_init[0] = info->lo_init[0];
+	lo->lo_init[1] = info->lo_init[1];
+	if (info->lo_encrypt_key_size) {
+		memcpy(lo->lo_encrypt_key, info->lo_encrypt_key,
+		       info->lo_encrypt_key_size);
+		lo->lo_key_owner = uid;
+	}
+
+	return 0;
+}
+
+static int
+loop_set_status(struct loop_device *lo, const struct loop_info64 *info)
+{
+	int err;
 	struct block_device *bdev;
+	kuid_t uid = current_uid();
 	bool partscan = false;
 	bool size_changed = false;
 	loff_t validated_size;
@@ -1299,10 +1355,6 @@  loop_set_status(struct loop_device *lo, const struct loop_info64 *info)
 		err = -ENXIO;
 		goto out_unlock;
 	}
-	if ((unsigned int) info->lo_encrypt_key_size > LO_KEY_SIZE) {
-		err = -EINVAL;
-		goto out_unlock;
-	}
 
 	if (lo->lo_offset != info->lo_offset ||
 	    lo->lo_sizelimit != info->lo_sizelimit) {
@@ -1330,54 +1382,10 @@  loop_set_status(struct loop_device *lo, const struct loop_info64 *info)
 		goto out_unfreeze;
 	}
 
-	err = loop_release_xfer(lo);
-	if (err)
-		goto out_unfreeze;
-
-	if (info->lo_encrypt_type) {
-		unsigned int type = info->lo_encrypt_type;
-
-		if (type >= MAX_LO_CRYPT) {
-			err = -EINVAL;
-			goto out_unfreeze;
-		}
-		xfer = xfer_funcs[type];
-		if (xfer == NULL) {
-			err = -EINVAL;
-			goto out_unfreeze;
-		}
-	} else
-		xfer = NULL;
-
-	err = loop_init_xfer(lo, xfer, info);
+	err = loop_set_from_status(lo, info);
 	if (err)
 		goto out_unfreeze;
 
-	lo->lo_offset = info->lo_offset;
-	lo->lo_sizelimit = info->lo_sizelimit;
-	memcpy(lo->lo_file_name, info->lo_file_name, LO_NAME_SIZE);
-	memcpy(lo->lo_crypt_name, info->lo_crypt_name, LO_NAME_SIZE);
-	lo->lo_file_name[LO_NAME_SIZE-1] = 0;
-	lo->lo_crypt_name[LO_NAME_SIZE-1] = 0;
-
-	if (!xfer)
-		xfer = &none_funcs;
-	lo->transfer = xfer->transfer;
-	lo->ioctl = xfer->ioctl;
-
-	if ((lo->lo_flags & LO_FLAGS_AUTOCLEAR) !=
-	     (info->lo_flags & LO_FLAGS_AUTOCLEAR))
-		lo->lo_flags ^= LO_FLAGS_AUTOCLEAR;
-
-	lo->lo_encrypt_key_size = info->lo_encrypt_key_size;
-	lo->lo_init[0] = info->lo_init[0];
-	lo->lo_init[1] = info->lo_init[1];
-	if (info->lo_encrypt_key_size) {
-		memcpy(lo->lo_encrypt_key, info->lo_encrypt_key,
-		       info->lo_encrypt_key_size);
-		lo->lo_key_owner = uid;
-	}
-
 	if (size_changed)
 		loop_set_size(lo, validated_size);