| Message ID | 20230919020806.534183-1-yanjun.zhu@intel.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [1/1] RDMA/rtrs: Fix the problem of variable not initialized fully | expand |
On Tue, Sep 19, 2023 at 10:08:06AM +0800, Zhu Yanjun wrote: > From: Zhu Yanjun <yanjun.zhu@linux.dev> > > No functionality change. The variable which is not initialized fully > will introduce potential risks. Are you sure about not being initialized? Thanks > > Signed-off-by: Zhu Yanjun <yanjun.zhu@linux.dev> > --- > drivers/infiniband/ulp/rtrs/rtrs.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/infiniband/ulp/rtrs/rtrs.c b/drivers/infiniband/ulp/rtrs/rtrs.c > index 3696f367ff51..d80edfffd2e4 100644 > --- a/drivers/infiniband/ulp/rtrs/rtrs.c > +++ b/drivers/infiniband/ulp/rtrs/rtrs.c > @@ -255,7 +255,7 @@ static int create_cq(struct rtrs_con *con, int cq_vector, int nr_cqe, > static int create_qp(struct rtrs_con *con, struct ib_pd *pd, > u32 max_send_wr, u32 max_recv_wr, u32 max_sge) > { > - struct ib_qp_init_attr init_attr = {NULL}; > + struct ib_qp_init_attr init_attr = {}; > struct rdma_cm_id *cm_id = con->cm_id; > int ret; > > -- > 2.40.1 >
在 2023/9/19 16:17, Leon Romanovsky 写道: > On Tue, Sep 19, 2023 at 10:08:06AM +0800, Zhu Yanjun wrote: >> From: Zhu Yanjun <yanjun.zhu@linux.dev> >> >> No functionality change. The variable which is not initialized fully >> will introduce potential risks. > Are you sure about not being initialized? About this problem, I think we discussed it previously in RDMA maillist. And at that time, IIRC, you shared a link with me. The link is as below. https://www.ex-parrot.com/~chris/random/initialise.html From what we discussed and the above link, I think it is not initialized fully. Zhu Yanjun > > Thanks > >> Signed-off-by: Zhu Yanjun <yanjun.zhu@linux.dev> >> --- >> drivers/infiniband/ulp/rtrs/rtrs.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/infiniband/ulp/rtrs/rtrs.c b/drivers/infiniband/ulp/rtrs/rtrs.c >> index 3696f367ff51..d80edfffd2e4 100644 >> --- a/drivers/infiniband/ulp/rtrs/rtrs.c >> +++ b/drivers/infiniband/ulp/rtrs/rtrs.c >> @@ -255,7 +255,7 @@ static int create_cq(struct rtrs_con *con, int cq_vector, int nr_cqe, >> static int create_qp(struct rtrs_con *con, struct ib_pd *pd, >> u32 max_send_wr, u32 max_recv_wr, u32 max_sge) >> { >> - struct ib_qp_init_attr init_attr = {NULL}; >> + struct ib_qp_init_attr init_attr = {}; >> struct rdma_cm_id *cm_id = con->cm_id; >> int ret; >> >> -- >> 2.40.1 >>
On Tue, Sep 19, 2023 at 04:26:54PM +0800, Zhu Yanjun wrote: > > 在 2023/9/19 16:17, Leon Romanovsky 写道: > > On Tue, Sep 19, 2023 at 10:08:06AM +0800, Zhu Yanjun wrote: > > > From: Zhu Yanjun <yanjun.zhu@linux.dev> > > > > > > No functionality change. The variable which is not initialized fully > > > will introduce potential risks. > > Are you sure about not being initialized? > > About this problem, I think we discussed it previously in RDMA maillist. > > And at that time, IIRC, you shared a link with me. The link is as below. > > https://www.ex-parrot.com/~chris/random/initialise.html > > From what we discussed and the above link, I think it is not initialized > fully. I remember that discussion and it was about slightly different thing: {} vs {0} in Linux kernel. However I don't think that I sent you that link. Anyway, let's take this patch as it is harmless. Thanks > > > Zhu Yanjun > > > > > Thanks > > > > > Signed-off-by: Zhu Yanjun <yanjun.zhu@linux.dev> > > > --- > > > drivers/infiniband/ulp/rtrs/rtrs.c | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/drivers/infiniband/ulp/rtrs/rtrs.c b/drivers/infiniband/ulp/rtrs/rtrs.c > > > index 3696f367ff51..d80edfffd2e4 100644 > > > --- a/drivers/infiniband/ulp/rtrs/rtrs.c > > > +++ b/drivers/infiniband/ulp/rtrs/rtrs.c > > > @@ -255,7 +255,7 @@ static int create_cq(struct rtrs_con *con, int cq_vector, int nr_cqe, > > > static int create_qp(struct rtrs_con *con, struct ib_pd *pd, > > > u32 max_send_wr, u32 max_recv_wr, u32 max_sge) > > > { > > > - struct ib_qp_init_attr init_attr = {NULL}; > > > + struct ib_qp_init_attr init_attr = {}; > > > struct rdma_cm_id *cm_id = con->cm_id; > > > int ret; > > > -- > > > 2.40.1 > > >
On Tue, 19 Sep 2023 10:08:06 +0800, Zhu Yanjun wrote: > No functionality change. The variable which is not initialized fully > will introduce potential risks. > > Applied, thanks! [1/1] RDMA/rtrs: Fix the problem of variable not initialized fully https://git.kernel.org/rdma/rdma/c/c5930a1aa08aaf Best regards,
On 19/09/2023 17:30, Leon Romanovsky wrote: > On Tue, Sep 19, 2023 at 04:26:54PM +0800, Zhu Yanjun wrote: >> >> 在 2023/9/19 16:17, Leon Romanovsky 写道: >>> On Tue, Sep 19, 2023 at 10:08:06AM +0800, Zhu Yanjun wrote: >>>> From: Zhu Yanjun <yanjun.zhu@linux.dev> >>>> >>>> No functionality change. The variable which is not initialized fully >>>> will introduce potential risks. >>> Are you sure about not being initialized? >> >> About this problem, I think we discussed it previously in RDMA maillist. >> >> And at that time, IIRC, you shared a link with me. The link is as below. >> >> https://www.ex-parrot.com/~chris/random/initialise.html >> >> From what we discussed and the above link, I think it is not initialized >> fully. > > I remember that discussion and it was about slightly different thing: > {} vs {0} in Linux kernel. Well, in my mind, I thought they are the same. see: https://www.gnu.org/software/gnu-c-manual/gnu-c-manual.html#Initializing-Structure-Members In current kernel, {NULL/0} is used in so many other places. beside {NULL}, another partial initializing form struct class { int a, b, c, d, e; } instance = { .a = x, .b = y, }; They are also used everywhere, it's definitely based on the truth instance.{c,d,e} to be "0". Thanks > > However I don't think that I sent you that link. > Anyway, let's take this patch as it is harmless. > > Thanks > >> >> >> Zhu Yanjun >> >>> >>> Thanks >>> >>>> Signed-off-by: Zhu Yanjun <yanjun.zhu@linux.dev> >>>> --- >>>> drivers/infiniband/ulp/rtrs/rtrs.c | 2 +- >>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>> >>>> diff --git a/drivers/infiniband/ulp/rtrs/rtrs.c b/drivers/infiniband/ulp/rtrs/rtrs.c >>>> index 3696f367ff51..d80edfffd2e4 100644 >>>> --- a/drivers/infiniband/ulp/rtrs/rtrs.c >>>> +++ b/drivers/infiniband/ulp/rtrs/rtrs.c >>>> @@ -255,7 +255,7 @@ static int create_cq(struct rtrs_con *con, int cq_vector, int nr_cqe, >>>> static int create_qp(struct rtrs_con *con, struct ib_pd *pd, >>>> u32 max_send_wr, u32 max_recv_wr, u32 max_sge) >>>> { >>>> - struct ib_qp_init_attr init_attr = {NULL}; >>>> + struct ib_qp_init_attr init_attr = {}; >>>> struct rdma_cm_id *cm_id = con->cm_id; >>>> int ret; >>>> -- >>>> 2.40.1 >>>>
On Wed, Sep 20, 2023 at 4:16 AM Zhijian Li (Fujitsu) <lizhijian@fujitsu.com> wrote: > > > > On 19/09/2023 17:30, Leon Romanovsky wrote: > > On Tue, Sep 19, 2023 at 04:26:54PM +0800, Zhu Yanjun wrote: > >> > >> 在 2023/9/19 16:17, Leon Romanovsky 写道: > >>> On Tue, Sep 19, 2023 at 10:08:06AM +0800, Zhu Yanjun wrote: > >>>> From: Zhu Yanjun <yanjun.zhu@linux.dev> > >>>> > >>>> No functionality change. The variable which is not initialized fully > >>>> will introduce potential risks. > >>> Are you sure about not being initialized? > >> > >> About this problem, I think we discussed it previously in RDMA maillist. > >> > >> And at that time, IIRC, you shared a link with me. The link is as below. > >> > >> https://www.ex-parrot.com/~chris/random/initialise.html > >> > >> From what we discussed and the above link, I think it is not initialized > >> fully. > > > > I remember that discussion and it was about slightly different thing: > > {} vs {0} in Linux kernel. > > > Well, in my mind, I thought they are the same. see: https://www.gnu.org/software/gnu-c-manual/gnu-c-manual.html#Initializing-Structure-Members > > In current kernel, {NULL/0} is used in so many other places. beside {NULL}, another partial initializing form > struct class { > int a, b, c, d, e; > } instance = { > .a = x, > .b = y, > }; > > They are also used everywhere, it's definitely based on the truth instance.{c,d,e} to be "0". I also think they are the same. oth it is harmless. Thx > > > Thanks > > > > > > > However I don't think that I sent you that link. > > Anyway, let's take this patch as it is harmless. > > > > Thanks > > > >> > >> > >> Zhu Yanjun > >> > >>> > >>> Thanks > >>> > >>>> Signed-off-by: Zhu Yanjun <yanjun.zhu@linux.dev> > >>>> --- > >>>> drivers/infiniband/ulp/rtrs/rtrs.c | 2 +- > >>>> 1 file changed, 1 insertion(+), 1 deletion(-) > >>>> > >>>> diff --git a/drivers/infiniband/ulp/rtrs/rtrs.c b/drivers/infiniband/ulp/rtrs/rtrs.c > >>>> index 3696f367ff51..d80edfffd2e4 100644 > >>>> --- a/drivers/infiniband/ulp/rtrs/rtrs.c > >>>> +++ b/drivers/infiniband/ulp/rtrs/rtrs.c > >>>> @@ -255,7 +255,7 @@ static int create_cq(struct rtrs_con *con, int cq_vector, int nr_cqe, > >>>> static int create_qp(struct rtrs_con *con, struct ib_pd *pd, > >>>> u32 max_send_wr, u32 max_recv_wr, u32 max_sge) > >>>> { > >>>> - struct ib_qp_init_attr init_attr = {NULL}; > >>>> + struct ib_qp_init_attr init_attr = {}; > >>>> struct rdma_cm_id *cm_id = con->cm_id; > >>>> int ret; > >>>> -- > >>>> 2.40.1 > >>>>
On Wed, Sep 20, 2023 at 02:16:20AM +0000, Zhijian Li (Fujitsu) wrote: > > > On 19/09/2023 17:30, Leon Romanovsky wrote: > > On Tue, Sep 19, 2023 at 04:26:54PM +0800, Zhu Yanjun wrote: > >> > >> 在 2023/9/19 16:17, Leon Romanovsky 写道: > >>> On Tue, Sep 19, 2023 at 10:08:06AM +0800, Zhu Yanjun wrote: > >>>> From: Zhu Yanjun <yanjun.zhu@linux.dev> > >>>> > >>>> No functionality change. The variable which is not initialized fully > >>>> will introduce potential risks. > >>> Are you sure about not being initialized? > >> > >> About this problem, I think we discussed it previously in RDMA maillist. > >> > >> And at that time, IIRC, you shared a link with me. The link is as below. > >> > >> https://www.ex-parrot.com/~chris/random/initialise.html > >> > >> From what we discussed and the above link, I think it is not initialized > >> fully. > > > > I remember that discussion and it was about slightly different thing: > > {} vs {0} in Linux kernel. > > > Well, in my mind, I thought they are the same. see: https://www.gnu.org/software/gnu-c-manual/gnu-c-manual.html#Initializing-Structure-Members It is GCC specific implementation, the original discussion was about C-standard. Thanks
On Wed, Sep 20, 2023 at 10:47:53AM +0300, Leon Romanovsky wrote: > > >> About this problem, I think we discussed it previously in RDMA maillist. > > >> > > >> And at that time, IIRC, you shared a link with me. The link is as below. > > >> > > >> https://www.ex-parrot.com/~chris/random/initialise.html > > >> > > >> From what we discussed and the above link, I think it is not initialized > > >> fully. > > > > > > I remember that discussion and it was about slightly different thing: > > > {} vs {0} in Linux kernel. > > > > > > Well, in my mind, I thought they are the same. see: https://www.gnu.org/software/gnu-c-manual/gnu-c-manual.html#Initializing-Structure-Members > > It is GCC specific implementation, the original discussion was about C-standard. Yes, the C standard says they are different constructs and pedantically only {} is required to fully zero the struct, padding and all. {0} says 'zero initialize the first member of the struct', it is a terrible construct because the first member may not be an integer, don't use it. Jason
diff --git a/drivers/infiniband/ulp/rtrs/rtrs.c b/drivers/infiniband/ulp/rtrs/rtrs.c index 3696f367ff51..d80edfffd2e4 100644 --- a/drivers/infiniband/ulp/rtrs/rtrs.c +++ b/drivers/infiniband/ulp/rtrs/rtrs.c @@ -255,7 +255,7 @@ static int create_cq(struct rtrs_con *con, int cq_vector, int nr_cqe, static int create_qp(struct rtrs_con *con, struct ib_pd *pd, u32 max_send_wr, u32 max_recv_wr, u32 max_sge) { - struct ib_qp_init_attr init_attr = {NULL}; + struct ib_qp_init_attr init_attr = {}; struct rdma_cm_id *cm_id = con->cm_id; int ret;