Re: Re: [PATCH v4 1/5] virtio-crypto: change code style
On 4/26/22 14:12, Jason Wang wrote:
On Sun, Apr 24, 2022 at 6:45 PM zhenwei pi wrote:
Use temporary variable to make code easy to read and maintain.
/* Pad cipher's parameters */
vcrypto->ctrl.u.sym_create_session.op_type =
cpu_to_le32(VIRTIO_CRYPTO_SYM_OP_CIPHER);
vcrypto->ctrl.u.sym_create_session.u.cipher.para.algo =
vcrypto->ctrl.header.algo;
vcrypto->ctrl.u.sym_create_session.u.cipher.para.keylen =
cpu_to_le32(keylen);
vcrypto->ctrl.u.sym_create_session.u.cipher.para.op =
cpu_to_le32(op);
-->
sym_create_session = >u.sym_create_session;
sym_create_session->op_type = cpu_to_le32(VIRTIO_CRYPTO_SYM_OP_CIPHER);
sym_create_session->u.cipher.para.algo = ctrl->header.algo;
sym_create_session->u.cipher.para.keylen = cpu_to_le32(keylen);
sym_create_session->u.cipher.para.op = cpu_to_le32(op);
The new style shows more obviously:
- the variable we want to operate.
- an assignment statement in a single line.
Still hundreds of lines of changes, I'd leave this change to other
mainters to dedice.
Thanks
Thanks to Jason!
Hi, Lei
What's your opinion?
Cc: Michael S. Tsirkin
Cc: Jason Wang
Cc: Gonglei
Signed-off-by: zhenwei pi
---
.../virtio/virtio_crypto_akcipher_algs.c | 40 ++-
.../virtio/virtio_crypto_skcipher_algs.c | 72 +--
2 files changed, 59 insertions(+), 53 deletions(-)
diff --git a/drivers/crypto/virtio/virtio_crypto_akcipher_algs.c
b/drivers/crypto/virtio/virtio_crypto_akcipher_algs.c
index f3ec9420215e..20901a263fc8 100644
--- a/drivers/crypto/virtio/virtio_crypto_akcipher_algs.c
+++ b/drivers/crypto/virtio/virtio_crypto_akcipher_algs.c
@@ -106,23 +106,27 @@ static int virtio_crypto_alg_akcipher_init_session(struct
virtio_crypto_akcipher
unsigned int inlen;
int err;
unsigned int num_out = 0, num_in = 0;
+ struct virtio_crypto_op_ctrl_req *ctrl;
+ struct virtio_crypto_session_input *input;
pkey = kmemdup(key, keylen, GFP_ATOMIC);
if (!pkey)
return -ENOMEM;
spin_lock(>ctrl_lock);
- memcpy(>ctrl.header, header, sizeof(vcrypto->ctrl.header));
- memcpy(>ctrl.u, para, sizeof(vcrypto->ctrl.u));
- vcrypto->input.status = cpu_to_le32(VIRTIO_CRYPTO_ERR);
+ ctrl = >ctrl;
+ memcpy(>header, header, sizeof(ctrl->header));
+ memcpy(>u, para, sizeof(ctrl->u));
+ input = >input;
+ input->status = cpu_to_le32(VIRTIO_CRYPTO_ERR);
- sg_init_one(_sg, >ctrl, sizeof(vcrypto->ctrl));
+ sg_init_one(_sg, ctrl, sizeof(*ctrl));
sgs[num_out++] = _sg;
sg_init_one(_sg, pkey, keylen);
sgs[num_out++] = _sg;
- sg_init_one(_sg, >input, sizeof(vcrypto->input));
+ sg_init_one(_sg, input, sizeof(*input));
sgs[num_out + num_in++] = _sg;
err = virtqueue_add_sgs(vcrypto->ctrl_vq, sgs, num_out, num_in,
vcrypto, GFP_ATOMIC);
@@ -134,12 +138,12 @@ static int virtio_crypto_alg_akcipher_init_session(struct
virtio_crypto_akcipher
!virtqueue_is_broken(vcrypto->ctrl_vq))
cpu_relax();
- if (le32_to_cpu(vcrypto->input.status) != VIRTIO_CRYPTO_OK) {
+ if (le32_to_cpu(input->status) != VIRTIO_CRYPTO_OK) {
err = -EINVAL;
goto out;
}
- ctx->session_id = le64_to_cpu(vcrypto->input.session_id);
+ ctx->session_id = le64_to_cpu(input->session_id);
ctx->session_valid = true;
err = 0;
@@ -149,7 +153,7 @@ static int virtio_crypto_alg_akcipher_init_session(struct
virtio_crypto_akcipher
if (err < 0)
pr_err("virtio_crypto: Create session failed status: %u\n",
- le32_to_cpu(vcrypto->input.status));
+ le32_to_cpu(input->status));
return err;
}
@@ -161,23 +165,27 @@ static int
virtio_crypto_alg_akcipher_close_session(struct virtio_crypto_akciphe
struct virtio_crypto *vcrypto = ctx->vcrypto;
unsigned int num_out = 0, num_in = 0, inlen;
int err;
+ struct virtio_crypto_op_ctrl_req *ctrl;
+ struct virtio_crypto_inhdr *ctrl_status;
spin_lock(>ctrl_lock);
if (!ctx->session_valid) {
err = 0;
goto out;
}
- vcrypto->ctrl_status.status = VIRTIO_CRYPTO_ERR;
- vcrypto->ctrl.header.opcode =
cpu_to_le32(VIRTIO_CRYPTO_AKCIPHER_DESTROY_SESSION);
- vcrypto->ctrl.header.queue_id = 0;
+ ctrl_status = >ctrl_status;
+ ctrl_status->status = VIRTIO_CRYPTO_ERR;
+ ctrl = >ctrl;
+ ctrl->header.opcode =
cpu_to_le32(VIRTIO_CRYPTO_AKCIPHER_DESTROY_SESSION);
+ ctrl->header.queue_id = 0;
- destroy_session = >ctrl.u.destroy_session;
+ destroy_session = >u.destroy_session;
destroy_session->session_id =
Re: [PATCH v4 1/5] virtio-crypto: change code style
On Sun, Apr 24, 2022 at 6:45 PM zhenwei pi wrote:
>
> Use temporary variable to make code easy to read and maintain.
> /* Pad cipher's parameters */
> vcrypto->ctrl.u.sym_create_session.op_type =
> cpu_to_le32(VIRTIO_CRYPTO_SYM_OP_CIPHER);
> vcrypto->ctrl.u.sym_create_session.u.cipher.para.algo =
> vcrypto->ctrl.header.algo;
> vcrypto->ctrl.u.sym_create_session.u.cipher.para.keylen =
> cpu_to_le32(keylen);
> vcrypto->ctrl.u.sym_create_session.u.cipher.para.op =
> cpu_to_le32(op);
> -->
> sym_create_session = >u.sym_create_session;
> sym_create_session->op_type =
> cpu_to_le32(VIRTIO_CRYPTO_SYM_OP_CIPHER);
> sym_create_session->u.cipher.para.algo = ctrl->header.algo;
> sym_create_session->u.cipher.para.keylen = cpu_to_le32(keylen);
> sym_create_session->u.cipher.para.op = cpu_to_le32(op);
>
> The new style shows more obviously:
> - the variable we want to operate.
> - an assignment statement in a single line.
Still hundreds of lines of changes, I'd leave this change to other
mainters to dedice.
Thanks
>
> Cc: Michael S. Tsirkin
> Cc: Jason Wang
> Cc: Gonglei
> Signed-off-by: zhenwei pi
> ---
> .../virtio/virtio_crypto_akcipher_algs.c | 40 ++-
> .../virtio/virtio_crypto_skcipher_algs.c | 72 +--
> 2 files changed, 59 insertions(+), 53 deletions(-)
>
> diff --git a/drivers/crypto/virtio/virtio_crypto_akcipher_algs.c
> b/drivers/crypto/virtio/virtio_crypto_akcipher_algs.c
> index f3ec9420215e..20901a263fc8 100644
> --- a/drivers/crypto/virtio/virtio_crypto_akcipher_algs.c
> +++ b/drivers/crypto/virtio/virtio_crypto_akcipher_algs.c
> @@ -106,23 +106,27 @@ static int
> virtio_crypto_alg_akcipher_init_session(struct virtio_crypto_akcipher
> unsigned int inlen;
> int err;
> unsigned int num_out = 0, num_in = 0;
> + struct virtio_crypto_op_ctrl_req *ctrl;
> + struct virtio_crypto_session_input *input;
>
> pkey = kmemdup(key, keylen, GFP_ATOMIC);
> if (!pkey)
> return -ENOMEM;
>
> spin_lock(>ctrl_lock);
> - memcpy(>ctrl.header, header, sizeof(vcrypto->ctrl.header));
> - memcpy(>ctrl.u, para, sizeof(vcrypto->ctrl.u));
> - vcrypto->input.status = cpu_to_le32(VIRTIO_CRYPTO_ERR);
> + ctrl = >ctrl;
> + memcpy(>header, header, sizeof(ctrl->header));
> + memcpy(>u, para, sizeof(ctrl->u));
> + input = >input;
> + input->status = cpu_to_le32(VIRTIO_CRYPTO_ERR);
>
> - sg_init_one(_sg, >ctrl, sizeof(vcrypto->ctrl));
> + sg_init_one(_sg, ctrl, sizeof(*ctrl));
> sgs[num_out++] = _sg;
>
> sg_init_one(_sg, pkey, keylen);
> sgs[num_out++] = _sg;
>
> - sg_init_one(_sg, >input, sizeof(vcrypto->input));
> + sg_init_one(_sg, input, sizeof(*input));
> sgs[num_out + num_in++] = _sg;
>
> err = virtqueue_add_sgs(vcrypto->ctrl_vq, sgs, num_out, num_in,
> vcrypto, GFP_ATOMIC);
> @@ -134,12 +138,12 @@ static int
> virtio_crypto_alg_akcipher_init_session(struct virtio_crypto_akcipher
>!virtqueue_is_broken(vcrypto->ctrl_vq))
> cpu_relax();
>
> - if (le32_to_cpu(vcrypto->input.status) != VIRTIO_CRYPTO_OK) {
> + if (le32_to_cpu(input->status) != VIRTIO_CRYPTO_OK) {
> err = -EINVAL;
> goto out;
> }
>
> - ctx->session_id = le64_to_cpu(vcrypto->input.session_id);
> + ctx->session_id = le64_to_cpu(input->session_id);
> ctx->session_valid = true;
> err = 0;
>
> @@ -149,7 +153,7 @@ static int virtio_crypto_alg_akcipher_init_session(struct
> virtio_crypto_akcipher
>
> if (err < 0)
> pr_err("virtio_crypto: Create session failed status: %u\n",
> - le32_to_cpu(vcrypto->input.status));
> + le32_to_cpu(input->status));
>
> return err;
> }
> @@ -161,23 +165,27 @@ static int
> virtio_crypto_alg_akcipher_close_session(struct virtio_crypto_akciphe
> struct virtio_crypto *vcrypto = ctx->vcrypto;
> unsigned int num_out = 0, num_in = 0, inlen;
> int err;
> + struct virtio_crypto_op_ctrl_req *ctrl;
> + struct virtio_crypto_inhdr *ctrl_status;
>
> spin_lock(>ctrl_lock);
> if (!ctx->session_valid) {
> err = 0;
> goto out;
> }
> - vcrypto->ctrl_status.status = VIRTIO_CRYPTO_ERR;
> - vcrypto->ctrl.header.opcode =
> cpu_to_le32(VIRTIO_CRYPTO_AKCIPHER_DESTROY_SESSION);
> - vcrypto->ctrl.header.queue_id = 0;
> + ctrl_status = >ctrl_status;
> + ctrl_status->status = VIRTIO_CRYPTO_ERR;
> + ctrl = >ctrl;
> + ctrl->header.opcode =
> cpu_to_le32(VIRTIO_CRYPTO_AKCIPHER_DESTROY_SESSION);
> + ctrl->header.queue_id = 0;
>
> - destroy_session = >ctrl.u.destroy_session;
> +
