On Tue, Jun 2, 2015 at 8:27 PM, Jason Gunthorpe
<jguntho...@obsidianresearch.com> wrote:
> On Tue, Jun 02, 2015 at 10:29:14AM +0300, Matan Barak wrote:
>> On Mon, Jun 1, 2015 at 7:53 PM, Jason Gunthorpe
>> <jguntho...@obsidianresearch.com> wrote:
>> > On Sun, May 31, 2015 at 03:14:10PM +0300, Or Gerlitz wrote:
>> >
>> >> +     struct ib_cq_init_attr cq_attr;
>> >>
>> >>       /* Create new device info */
>> >>       port_priv = kzalloc(sizeof *port_priv, GFP_KERNEL);
>> >> @@ -2943,9 +2944,11 @@ static int ib_mad_port_open(struct ib_device 
>> >> *device,
>> >>       if (has_smi)
>> >>               cq_size *= 2;
>> >>
>> >> +     memset(&cq_attr, 0, sizeof(cq_attr));
>> >> +     cq_attr.cqe = cq_size;
>> >
>> > Why does this patch switch to using memset when the prior patch used
>> > = {} ?
>> >
>>
>> Why does it matter? Both are valid approaches, aren't they?
>
> Sure, but why mix and match techniques in the same code base? Is there
> a reason?
>

No reason. We'll change that to {} style.

>> >> @@ -1075,12 +1075,11 @@ EXPORT_SYMBOL(ib_destroy_qp);
>> >>  struct ib_cq *ib_create_cq(struct ib_device *device,
>> >>                          ib_comp_handler comp_handler,
>> >>                          void (*event_handler)(struct ib_event *, void *),
>> >> -                        void *cq_context, int cqe, int comp_vector)
>> >> +                        void *cq_context, struct ib_cq_init_attr 
>> >> *cq_attr)
>> >>  {
>> >>       struct ib_cq *cq;
>> >> -     struct ib_cq_init_attr attr = {.cqe = cqe, .comp_vector = 
>> >> comp_vector};
>> >>
>> >> -     cq = device->create_cq(device, &attr, NULL, NULL);
>> >> +     cq = device->create_cq(device, cq_attr, NULL, NULL);
>> >
>> > How does this compile without warnings?
>> >
>>
>> Do you mean that there's a missing const here? It doesn't (and shouldn't)
>> cause warnings.
>
> Warnings will be emitted when you compile with -Wcast-qual (make W=3).
>
> Some of the static tools may warn/error on implicitly casting away
> const, I'm not sure.
>
> It is broadly undesirable to implicitly cast away const, even if the
> Kernel warning defaults don't produce the message.
>

Why is it casting away the const?
ib_create_cq gets a "struct ib_cq_init_attr *" and passes it to
device->create_cq which gets a "const struct ib_cq_init_attr *",
so I'm adding a const, which is perfectly fine.

Anyway, we'll add const to ib_create_cq as well and run make W=3 to verify.

> Jason

Thanks for your comments.

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

Reply via email to