diff mbox

[05/47] RDMA/uverbs: Make the code in ib_uverbs_cmd_verbs() less confusing

Message ID 20171006213333.6721-6-bart.vanassche@wdc.com (mailing list archive)
State Superseded
Headers show

Commit Message

Bart Van Assche Oct. 6, 2017, 9:32 p.m. UTC
This patch reduces the number of #ifdefs and also avoids that
smatch reports the following:

drivers/infiniband/core/uverbs_ioctl.c:276: ib_uverbs_cmd_verbs() warn: if statement not indented
drivers/infiniband/core/uverbs_ioctl.c:280: ib_uverbs_cmd_verbs() warn: possible memory leak of 'ctx'
drivers/infiniband/core/uverbs_ioctl.c:315: ib_uverbs_cmd_verbs() warn: if statement not indented

References: commit fac9658cabb9 ("IB/core: Add new ioctl interface")
Signed-off-by: Bart Van Assche <bart.vanassche@wdc.com>
Cc: Matan Barak <matanb@mellanox.com>
Cc: Yishai Hadas <yishaih@mellanox.com>
---
 drivers/infiniband/core/uverbs_ioctl.c | 20 +++++++++-----------
 1 file changed, 9 insertions(+), 11 deletions(-)

Comments

Matan Barak Oct. 9, 2017, 7:40 a.m. UTC | #1
On 07/10/2017 00:32, Bart Van Assche wrote:
> This patch reduces the number of #ifdefs and also avoids that
> smatch reports the following:
>
> drivers/infiniband/core/uverbs_ioctl.c:276: ib_uverbs_cmd_verbs() warn: if statement not indented
> drivers/infiniband/core/uverbs_ioctl.c:280: ib_uverbs_cmd_verbs() warn: possible memory leak of 'ctx'
> drivers/infiniband/core/uverbs_ioctl.c:315: ib_uverbs_cmd_verbs() warn: if statement not indented
>
> References: commit fac9658cabb9 ("IB/core: Add new ioctl interface")
> Signed-off-by: Bart Van Assche <bart.vanassche@wdc.com>
> Cc: Matan Barak <matanb@mellanox.com>
> Cc: Yishai Hadas <yishaih@mellanox.com>
> ---
>   drivers/infiniband/core/uverbs_ioctl.c | 20 +++++++++-----------
>   1 file changed, 9 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/infiniband/core/uverbs_ioctl.c b/drivers/infiniband/core/uverbs_ioctl.c
> index 5286ad57d903..53f780ea49fe 100644
> --- a/drivers/infiniband/core/uverbs_ioctl.c
> +++ b/drivers/infiniband/core/uverbs_ioctl.c
> @@ -234,7 +234,7 @@ static long ib_uverbs_cmd_verbs(struct ib_device *ib_dev,
>       const struct uverbs_method_spec *method_spec;
>       long err = 0;
>       unsigned int i;
> -    struct {
> +    struct attr_and_bundle {
>           struct ib_uverbs_attr        *uattrs;
>           struct uverbs_attr_bundle    *uverbs_attr_bundle;
>       } *ctx = NULL;
> @@ -242,7 +242,10 @@ static long ib_uverbs_cmd_verbs(struct ib_device *ib_dev,
>       unsigned long *curr_bitmap;
>       size_t ctx_size;
>   #ifdef UVERBS_OPTIMIZE_USING_STACK_SZ
> -    uintptr_t data[UVERBS_OPTIMIZE_USING_STACK_SZ / sizeof(uintptr_t)];
> +    uintptr_t __data[UVERBS_OPTIMIZE_USING_STACK_SZ / sizeof(uintptr_t)];
> +    struct attr_and_bundle *const data = (void *)__data;
> +#else
> +    struct attr_and_bundle *const data = NULL;
>   #endif
>
>       if (hdr->reserved)
> @@ -269,13 +272,10 @@ static long ib_uverbs_cmd_verbs(struct ib_device *ib_dev,
>               (method_spec->num_child_attrs / BITS_PER_LONG +
>                method_spec->num_buckets);
>
> -#ifdef UVERBS_OPTIMIZE_USING_STACK_SZ
>       if (ctx_size <= UVERBS_OPTIMIZE_USING_STACK_SZ)

If UVERBS_OPTIMIZE_USING_STACK_SZ isn't defined, you'll get an error here.
Maybe we could define it as 0 in the else part of the local variables deceleration.

> -        ctx = (void *)data;
> -
> +        ctx = data;
>       if (!ctx)
> -#endif
> -    ctx = kmalloc(ctx_size, GFP_KERNEL);
> +        ctx = kmalloc(ctx_size, GFP_KERNEL);
>       if (!ctx)
>           return -ENOMEM;
>
> @@ -311,10 +311,8 @@ static long ib_uverbs_cmd_verbs(struct ib_device *ib_dev,
>       err = uverbs_handle_method(buf, ctx->uattrs, hdr->num_attrs, ib_dev,
>                      file, method_spec, ctx->uverbs_attr_bundle);
>   out:
> -#ifdef UVERBS_OPTIMIZE_USING_STACK_SZ
> -    if (ctx_size > UVERBS_OPTIMIZE_USING_STACK_SZ)
> -#endif
> -    kfree(ctx);
> +    if (ctx != data)
> +        kfree(ctx);
>       return err;
>   }
>
>

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/infiniband/core/uverbs_ioctl.c b/drivers/infiniband/core/uverbs_ioctl.c
index 5286ad57d903..53f780ea49fe 100644
--- a/drivers/infiniband/core/uverbs_ioctl.c
+++ b/drivers/infiniband/core/uverbs_ioctl.c
@@ -234,7 +234,7 @@  static long ib_uverbs_cmd_verbs(struct ib_device *ib_dev,
 	const struct uverbs_method_spec *method_spec;
 	long err = 0;
 	unsigned int i;
-	struct {
+	struct attr_and_bundle {
 		struct ib_uverbs_attr		*uattrs;
 		struct uverbs_attr_bundle	*uverbs_attr_bundle;
 	} *ctx = NULL;
@@ -242,7 +242,10 @@  static long ib_uverbs_cmd_verbs(struct ib_device *ib_dev,
 	unsigned long *curr_bitmap;
 	size_t ctx_size;
 #ifdef UVERBS_OPTIMIZE_USING_STACK_SZ
-	uintptr_t data[UVERBS_OPTIMIZE_USING_STACK_SZ / sizeof(uintptr_t)];
+	uintptr_t __data[UVERBS_OPTIMIZE_USING_STACK_SZ / sizeof(uintptr_t)];
+	struct attr_and_bundle *const data = (void *)__data;
+#else
+	struct attr_and_bundle *const data = NULL;
 #endif
 
 	if (hdr->reserved)
@@ -269,13 +272,10 @@  static long ib_uverbs_cmd_verbs(struct ib_device *ib_dev,
 			(method_spec->num_child_attrs / BITS_PER_LONG +
 			 method_spec->num_buckets);
 
-#ifdef UVERBS_OPTIMIZE_USING_STACK_SZ
 	if (ctx_size <= UVERBS_OPTIMIZE_USING_STACK_SZ)
-		ctx = (void *)data;
-
+		ctx = data;
 	if (!ctx)
-#endif
-	ctx = kmalloc(ctx_size, GFP_KERNEL);
+		ctx = kmalloc(ctx_size, GFP_KERNEL);
 	if (!ctx)
 		return -ENOMEM;
 
@@ -311,10 +311,8 @@  static long ib_uverbs_cmd_verbs(struct ib_device *ib_dev,
 	err = uverbs_handle_method(buf, ctx->uattrs, hdr->num_attrs, ib_dev,
 				   file, method_spec, ctx->uverbs_attr_bundle);
 out:
-#ifdef UVERBS_OPTIMIZE_USING_STACK_SZ
-	if (ctx_size > UVERBS_OPTIMIZE_USING_STACK_SZ)
-#endif
-	kfree(ctx);
+	if (ctx != data)
+		kfree(ctx);
 	return err;
 }