On Mon, 2017-10-09 at 10:40 +0300, Matan Barak (External) wrote:
> 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@xxxxxxx>
> > Cc: Matan Barak <matanb@xxxxxxxxxxxx>
> > Cc: Yishai Hadas <yishaih@xxxxxxxxxxxx>
> > ---
> > 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.
Hello Matan,
That's an interesting suggestion but that wouldn't work well with the "#ifdef
UVERBS_OPTIMIZE_USING_STACK_SZ" code in the declaration section. How about
changing "if (ctx_size <= UVERBS_OPTIMIZE_USING_STACK_SZ) ctx = data;" into
"ctx = data;" such that it becomes again safe not to define UVERBS_OPTIMIZE_USING_STACK_SZ?
With that change this patch looks as follows:
diff --git a/drivers/infiniband/core/uverbs_ioctl.c b/drivers/infiniband/core/uverbs_ioctl.c
index 5286ad57d903..81c05ba2a58b 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,9 @@ 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 +310,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;
}
��.n��������+%������w��{.n�����{���fk��ܨ}���Ơz�j:+v�����w����ޙ��&�)ߡ�a����z�ޗ���ݢj��w�f
