2020-07-20 07:21:16

by yekai (A)

Subject: [PATCH] uacce: fix some coding styles

1. add some parameter check.
2. delete some redundant code.
3. modify the module author information.

Signed-off-by: Kai Ye <[email protected]>
Reviewed-by: Zhou Wang <[email protected]>
---
drivers/misc/uacce/uacce.c | 28 +++++++++++++---------------
1 file changed, 13 insertions(+), 15 deletions(-)

diff --git a/drivers/misc/uacce/uacce.c b/drivers/misc/uacce/uacce.c
index 107028e..2e1af58 100644
--- a/drivers/misc/uacce/uacce.c
+++ b/drivers/misc/uacce/uacce.c
@@ -63,8 +63,12 @@ static long uacce_fops_unl_ioctl(struct file *filep,
unsigned int cmd, unsigned long arg)
{
struct uacce_queue *q = filep->private_data;
- struct uacce_device *uacce = q->uacce;
+ struct uacce_device *uacce;
+
+ if (WARN_ON(!q))
+ return -EINVAL;

+ uacce = q->uacce;
switch (cmd) {
case UACCE_CMD_START_Q:
return uacce_start_queue(q);
@@ -206,11 +210,16 @@ static const struct vm_operations_struct uacce_vm_ops = {
static int uacce_fops_mmap(struct file *filep, struct vm_area_struct *vma)
{
struct uacce_queue *q = filep->private_data;
- struct uacce_device *uacce = q->uacce;
- struct uacce_qfile_region *qfr;
enum uacce_qfrt type = UACCE_MAX_REGION;
+ struct uacce_qfile_region *qfr;
+ struct uacce_device *uacce;
int ret = 0;

+ if (WARN_ON(!q))
+ return -EINVAL;
+
+ uacce = q->uacce;
+
if (vma->vm_pgoff < UACCE_MAX_REGION)
type = vma->vm_pgoff;
else
@@ -239,17 +248,6 @@ static int uacce_fops_mmap(struct file *filep, struct vm_area_struct *vma)

switch (type) {
case UACCE_QFRT_MMIO:
- if (!uacce->ops->mmap) {
- ret = -EINVAL;
- goto out_with_lock;
- }
-
- ret = uacce->ops->mmap(q, vma, qfr);
- if (ret)
- goto out_with_lock;
-
- break;
-
case UACCE_QFRT_DUS:
if (!uacce->ops->mmap) {
ret = -EINVAL;
@@ -541,5 +539,5 @@ subsys_initcall(uacce_init);
module_exit(uacce_exit);

MODULE_LICENSE("GPL");
-MODULE_AUTHOR("Hisilicon Tech. Co., Ltd.");
+MODULE_AUTHOR("HiSilicon Tech. Co., Ltd.");
MODULE_DESCRIPTION("Accelerator interface for Userland applications");
--
2.8.1

2020-07-21 07:01:38

by zhangfei

Subject: Re: [PATCH] uacce: fix some coding styles

On 2020/7/20 下午3:18, Kai Ye wrote:
> 1. add some parameter check.
> 2. delete some redundant code.
> 3. modify the module author information.
>
> Signed-off-by: Kai Ye <[email protected]>
> Reviewed-by: Zhou Wang <[email protected]>
Thanks Kai.
> ---
> drivers/misc/uacce/uacce.c | 28 +++++++++++++---------------
> 1 file changed, 13 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/misc/uacce/uacce.c b/drivers/misc/uacce/uacce.c
> index 107028e..2e1af58 100644
> --- a/drivers/misc/uacce/uacce.c
> +++ b/drivers/misc/uacce/uacce.c
> @@ -63,8 +63,12 @@ static long uacce_fops_unl_ioctl(struct file *filep,
> unsigned int cmd, unsigned long arg)
> {
> struct uacce_queue *q = filep->private_data;
> - struct uacce_device *uacce = q->uacce;
> + struct uacce_device *uacce;
> +
> + if (WARN_ON(!q))
> + return -EINVAL;
WARN_ON should not be used in uacce, instead error can be printed in
user space driver.
Error should not be printed in kernel log as pasid can be used by unpriv
user.

And I think we do not need check filep->private_data.
The fd is double checked in __fget_files.

Thanks