Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [PATCH 2/4] loop: Factor out configuring loop from status.
To:     Martijn Coenen <maco@android.com>, axboe@kernel.dk, hch@lst.de,
        ming.lei@redhat.com
Cc:     narayan@google.com, zezeozue@google.com, kernel-team@android.com,
        linux-block@vger.kernel.org, linux-kernel@vger.kernel.org,
        maco@google.com, Chaitanya.Kulkarni@wdc.com
References: <20200420080409.111693-1-maco@android.com>
 <20200420080409.111693-3-maco@android.com>
From:   Bart Van Assche <bvanassche@acm.org>
Message-ID: <50090c2e-d882-507f-43a7-5ed76c36390c@acm.org>
Date:   Mon, 20 Apr 2020 06:34:24 -0700
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101
 Thunderbird/68.6.0
MIME-Version: 1.0
In-Reply-To: <20200420080409.111693-3-maco@android.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

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.