| Message ID | 20190819140257.19319-1-bmt@zurich.ibm.com (mailing list archive) |
|---|---|
| State | Mainlined |
| Commit | 9b440078017f194e56eaae3ac32f333f420c5c4e |
| Headers | show |
| Series | siw: Fix potential NULL pointer in siw_connect(). | expand |
On Mon, 2019-08-19 at 16:02 +0200, Bernard Metzler wrote: > Fixes: 6c52fdc244b5 ("rdma/siw: connection management") > Reported-by: Dan Carpenter <dan.carpenter@oracle.com> > Signed-off-by: Bernard Metzler <bmt@zurich.ibm.com> > --- > drivers/infiniband/sw/siw/siw_cm.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/drivers/infiniband/sw/siw/siw_cm.c > b/drivers/infiniband/sw/siw/siw_cm.c > index 9ce8a1b925d2..fc97571a640b 100644 > --- a/drivers/infiniband/sw/siw/siw_cm.c > +++ b/drivers/infiniband/sw/siw/siw_cm.c > @@ -1515,7 +1515,7 @@ int siw_connect(struct iw_cm_id *id, struct > iw_cm_conn_param *params) > } > } > error: > - siw_dbg_qp(qp, "failed: %d\n", rv); > + siw_dbg(id->device, "failed: %d\n", rv); > > if (cep) { > siw_socket_disassoc(s); > @@ -1540,7 +1540,8 @@ int siw_connect(struct iw_cm_id *id, struct > iw_cm_conn_param *params) > } else if (s) { > sock_release(s); > } > - siw_qp_put(qp); > + if (qp) > + siw_qp_put(qp); > > return rv; > } Hi Bernard, We try to avoid empty commit messages. I know in this case the subject really carries the important info, but it looks better when displaying the commit if there is something in the message body. Also, please preface the subject with RDMA/siw: as we like to have the RDMA subsystem preface the individual subcomponent element in the subject line (some people still use IB, which is what used to be preferred, but RDMA is preferred today). I fixed both of those things up and applied this to for-rc, thanks. Please take a look (I pushed it out to my wip/dl-for-rc branch) so you can see what I mean about how to make both a simple subject line and a decent commit message. Also, no final punctuation on the subject line, and try to keep the subject length <= 50 chars total. If you have to go over to have a decent subject, then so be it, but we strive for that 50 char limit to make a subject stay on one line when displayed using git log --oneline.
-----"Doug Ledford" <dledford@redhat.com> wrote: ----- >To: "Bernard Metzler" <bmt@zurich.ibm.com>, >linux-rdma@vger.kernel.org >From: "Doug Ledford" <dledford@redhat.com> >Date: 08/20/2019 06:05PM >Cc: dan.carpenter@oracle.com, jgg@ziepe.ca >Subject: [EXTERNAL] Re: [PATCH] siw: Fix potential NULL pointer in >siw_connect(). > >On Mon, 2019-08-19 at 16:02 +0200, Bernard Metzler wrote: >> Fixes: 6c52fdc244b5 ("rdma/siw: connection management") >> Reported-by: Dan Carpenter <dan.carpenter@oracle.com> >> Signed-off-by: Bernard Metzler <bmt@zurich.ibm.com> >> --- >> drivers/infiniband/sw/siw/siw_cm.c | 5 +++-- >> 1 file changed, 3 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/infiniband/sw/siw/siw_cm.c >> b/drivers/infiniband/sw/siw/siw_cm.c >> index 9ce8a1b925d2..fc97571a640b 100644 >> --- a/drivers/infiniband/sw/siw/siw_cm.c >> +++ b/drivers/infiniband/sw/siw/siw_cm.c >> @@ -1515,7 +1515,7 @@ int siw_connect(struct iw_cm_id *id, struct >> iw_cm_conn_param *params) >> } >> } >> error: >> - siw_dbg_qp(qp, "failed: %d\n", rv); >> + siw_dbg(id->device, "failed: %d\n", rv); >> >> if (cep) { >> siw_socket_disassoc(s); >> @@ -1540,7 +1540,8 @@ int siw_connect(struct iw_cm_id *id, struct >> iw_cm_conn_param *params) >> } else if (s) { >> sock_release(s); >> } >> - siw_qp_put(qp); >> + if (qp) >> + siw_qp_put(qp); >> >> return rv; >> } > >Hi Bernard, > >We try to avoid empty commit messages. I know in this case the >subject >really carries the important info, but it looks better when >displaying >the commit if there is something in the message body. > >Also, please preface the subject with RDMA/siw: as we like to have >the >RDMA subsystem preface the individual subcomponent element in the >subject line (some people still use IB, which is what used to be >preferred, but RDMA is preferred today). > >I fixed both of those things up and applied this to for-rc, thanks. > >Please take a look (I pushed it out to my wip/dl-for-rc branch) so >you >can see what I mean about how to make both a simple subject line and >a >decent commit message. Also, no final punctuation on the subject >line, >and try to keep the subject length <= 50 chars total. If you have to >go >over to have a decent subject, then so be it, but we strive for that >50 >char limit to make a subject stay on one line when displayed using >git >log --oneline. > Hi Doug, I'll print that email and put it at the wall, right above the screen. Thanks so much for making it clear even to people like me! Best regards, Bernard
On Tue, Aug 20, 2019 at 12:05:33PM -0400, Doug Ledford wrote: > Please take a look (I pushed it out to my wip/dl-for-rc branch) so you > can see what I mean about how to make both a simple subject line and a > decent commit message. Also, no final punctuation on the subject line, > and try to keep the subject length <= 50 chars total. If you have to go > over to have a decent subject, then so be it, but we strive for that 50 > char limit to make a subject stay on one line when displayed using git > log --oneline. 50 is really small. If it were based on git log --oneline output the limit would be 67 characters. If you look at actual kernel git commits then the average subject is 52.4 characters and probably the upper bound is 60+ or so. I was surprised how well I had done personally at generating subjects when I looked at my own git log. My shortest subject was commit 0746556beab1 ("bna: off by one"). That was from 10 years ago and is not up to my current standards. My longest was commit 49d3d6c37a32 ("drivers/misc/sgi-gru/grukdump.c: unlocking should be conditional in gru_dump_context()"), but originally I used "gru:" as the patch prefix and Andrew changed it. :P regards, dan carpenter
On Wed, 2019-08-21 at 15:56 +0300, Dan Carpenter wrote: > On Tue, Aug 20, 2019 at 12:05:33PM -0400, Doug Ledford wrote: > > Please take a look (I pushed it out to my wip/dl-for-rc branch) so > > you > > can see what I mean about how to make both a simple subject line and > > a > > decent commit message. Also, no final punctuation on the subject > > line, > > and try to keep the subject length <= 50 chars total. If you have > > to go > > over to have a decent subject, then so be it, but we strive for that > > 50 > > char limit to make a subject stay on one line when displayed using > > git > > log --oneline. > > 50 is really small. 50 is the vim syntax highlighting suggested limit. You can go over, which is why I indicated it was a soft limit, but there you are. It leaves room for the displayed hash length to grow as well. > If it were based on git log --oneline output the > limit would be 67 characters. Only if you don't include room for the hash size to grow and other possible things, like a tag. > If you look at actual kernel git commits > then the average subject is 52.4 characters and probably the upper > bound > is 60+ or so. Yep, probably largely due to that very same vim syntax highlighting :-) > I was surprised how well I had done personally at generating subjects > when I looked at my own git log. > > My shortest subject was commit 0746556beab1 ("bna: off by one"). That > was from 10 years ago and is not up to my current standards. My > longest > was commit 49d3d6c37a32 ("drivers/misc/sgi-gru/grukdump.c: unlocking > should be conditional in gru_dump_context()"), but originally I used > "gru:" as the patch prefix and Andrew changed it. :P > > regards, > dan carpenter
On Wed, Aug 21, 2019 at 09:39:50AM -0400, Doug Ledford wrote: > On Wed, 2019-08-21 at 15:56 +0300, Dan Carpenter wrote: > > On Tue, Aug 20, 2019 at 12:05:33PM -0400, Doug Ledford wrote: > > > Please take a look (I pushed it out to my wip/dl-for-rc branch) so > > > you > > > can see what I mean about how to make both a simple subject line and > > > a > > > decent commit message. Also, no final punctuation on the subject > > > line, > > > and try to keep the subject length <= 50 chars total. If you have > > > to go > > > over to have a decent subject, then so be it, but we strive for that > > > 50 > > > char limit to make a subject stay on one line when displayed using > > > git > > > log --oneline. > > > > 50 is really small. > > 50 is the vim syntax highlighting suggested limit. You can go over, > which is why I indicated it was a soft limit, but there you are. It > leaves room for the displayed hash length to grow as well. I use 75 for all text in the commit message, as per Documentation/process/submitting-patches.rst People using 'git log --oneline' should have terminals wider than 80 :) The bigger question is if the first character after the subject tag should be uppper case or lower case <hum> Jason
On Wed, Aug 21, 2019 at 11:12:25AM -0300, Jason Gunthorpe wrote: > On Wed, Aug 21, 2019 at 09:39:50AM -0400, Doug Ledford wrote: > > On Wed, 2019-08-21 at 15:56 +0300, Dan Carpenter wrote: > > > On Tue, Aug 20, 2019 at 12:05:33PM -0400, Doug Ledford wrote: > > > > Please take a look (I pushed it out to my wip/dl-for-rc branch) so > > > > you > > > > can see what I mean about how to make both a simple subject line and > > > > a > > > > decent commit message. Also, no final punctuation on the subject > > > > line, > > > > and try to keep the subject length <= 50 chars total. If you have > > > > to go > > > > over to have a decent subject, then so be it, but we strive for that > > > > 50 > > > > char limit to make a subject stay on one line when displayed using > > > > git > > > > log --oneline. > > > > > > 50 is really small. > > > > 50 is the vim syntax highlighting suggested limit. You can go over, > > which is why I indicated it was a soft limit, but there you are. It > > leaves room for the displayed hash length to grow as well. > > I use 75 for all text in the commit message, as per > Documentation/process/submitting-patches.rst checkpatch.pl will give warning if you have subject line above 75 chars. > > People using 'git log --oneline' should have terminals wider than 80 > :) I like small terminals :(, it fits nicely with tiled WM on my laptop screen. > > The bigger question is if the first character after the subject tag > should be uppper case or lower case <hum> It doesn't matter as long as person submitting patches is consistent. Thanks > > Jason
On 21/08/2019 17:12, Jason Gunthorpe wrote: > On Wed, Aug 21, 2019 at 09:39:50AM -0400, Doug Ledford wrote: >> On Wed, 2019-08-21 at 15:56 +0300, Dan Carpenter wrote: >>> On Tue, Aug 20, 2019 at 12:05:33PM -0400, Doug Ledford wrote: >>>> Please take a look (I pushed it out to my wip/dl-for-rc branch) so >>>> you >>>> can see what I mean about how to make both a simple subject line and >>>> a >>>> decent commit message. Also, no final punctuation on the subject >>>> line, >>>> and try to keep the subject length <= 50 chars total. If you have >>>> to go >>>> over to have a decent subject, then so be it, but we strive for that >>>> 50 >>>> char limit to make a subject stay on one line when displayed using >>>> git >>>> log --oneline. >>> >>> 50 is really small. >> >> 50 is the vim syntax highlighting suggested limit. You can go over, >> which is why I indicated it was a soft limit, but there you are. It >> leaves room for the displayed hash length to grow as well. > > I use 75 for all text in the commit message, as per > Documentation/process/submitting-patches.rst > > People using 'git log --oneline' should have terminals wider than 80 > :) > > The bigger question is if the first character after the subject tag > should be uppper case or lower case <hum> I was thinking about that lately as well, it seems like git patches (which are pretty similar to the kernel) use lower-case letter [1]. RDMA subsystem mostly sticks to capital letter though: $ git log --oneline -- drivers/infiniband/ | egrep ": [a-z]" | wc -l 1364 $ git log --oneline -- drivers/infiniband/ | egrep ": [A-Z]" | wc -l 8069 Things look different when checking the entire tree: $ git log --oneline | egrep ": [a-z]" | wc -l 514596 $ git log --oneline | egrep ": [A-Z]" | wc -l 356939 [1] https://github.com/git/git/blob/master/Documentation/SubmittingPatches#L118
On Wed, Aug 21, 2019 at 11:12:25AM -0300, Jason Gunthorpe wrote: > On Wed, Aug 21, 2019 at 09:39:50AM -0400, Doug Ledford wrote: > > On Wed, 2019-08-21 at 15:56 +0300, Dan Carpenter wrote: > > > On Tue, Aug 20, 2019 at 12:05:33PM -0400, Doug Ledford wrote: > > > > Please take a look (I pushed it out to my wip/dl-for-rc branch) so > > > > you > > > > can see what I mean about how to make both a simple subject line and > > > > a > > > > decent commit message. Also, no final punctuation on the subject > > > > line, > > > > and try to keep the subject length <= 50 chars total. If you have > > > > to go > > > > over to have a decent subject, then so be it, but we strive for that > > > > 50 > > > > char limit to make a subject stay on one line when displayed using > > > > git > > > > log --oneline. > > > > > > 50 is really small. > > > > 50 is the vim syntax highlighting suggested limit. You can go over, > > which is why I indicated it was a soft limit, but there you are. It > > leaves room for the displayed hash length to grow as well. > > I use 75 for all text in the commit message, as per > Documentation/process/submitting-patches.rst > My limit is 57 characters for the subject (because otherwise mutt introduces a newline). I sometimes go over but I'm annoyed when forced to do that. 72 characters for the commit message because that's my limit for emails. > People using 'git log --oneline' should have terminals wider than 80 > :) > > The bigger question is if the first character after the subject tag > should be uppper case or lower case <hum> I feel like more and more people are moving to upper case. There are some people who insist on upper case and no one who insists on lower case so it's easier to just make everything upper case. regards, dan carpenter
On Thu, Aug 22, 2019 at 10:10:23AM +0300, Dan Carpenter wrote: > > People using 'git log --oneline' should have terminals wider than 80 > > :) > > > > The bigger question is if the first character after the subject tag > > should be uppper case or lower case <hum> > > I feel like more and more people are moving to upper case. There are > some people who insist on upper case and no one who insists on lower > case so it's easier to just make everything upper case. I've noticed Andrew lower cases everything going through his tree. I've always used upper case for no prarticular reason Jason
On Thu, 2019-08-22 at 09:02 -0300, Jason Gunthorpe wrote: > On Thu, Aug 22, 2019 at 10:10:23AM +0300, Dan Carpenter wrote: > > > > People using 'git log --oneline' should have terminals wider than > > > 80 > > > :) > > > > > > The bigger question is if the first character after the subject > > > tag > > > should be uppper case or lower case <hum> > > > > I feel like more and more people are moving to upper case. There > > are > > some people who insist on upper case and no one who insists on lower > > case so it's easier to just make everything upper case. > > I've noticed Andrew lower cases everything going through his > tree. I've always used upper case for no prarticular reason I prefer uppercase for the subsystem, and lowercase for the component.
diff --git a/drivers/infiniband/sw/siw/siw_cm.c b/drivers/infiniband/sw/siw/siw_cm.c index 9ce8a1b925d2..fc97571a640b 100644 --- a/drivers/infiniband/sw/siw/siw_cm.c +++ b/drivers/infiniband/sw/siw/siw_cm.c @@ -1515,7 +1515,7 @@ int siw_connect(struct iw_cm_id *id, struct iw_cm_conn_param *params) } } error: - siw_dbg_qp(qp, "failed: %d\n", rv); + siw_dbg(id->device, "failed: %d\n", rv); if (cep) { siw_socket_disassoc(s); @@ -1540,7 +1540,8 @@ int siw_connect(struct iw_cm_id *id, struct iw_cm_conn_param *params) } else if (s) { sock_release(s); } - siw_qp_put(qp); + if (qp) + siw_qp_put(qp); return rv; }
Fixes: 6c52fdc244b5 ("rdma/siw: connection management") Reported-by: Dan Carpenter <dan.carpenter@oracle.com> Signed-off-by: Bernard Metzler <bmt@zurich.ibm.com> --- drivers/infiniband/sw/siw/siw_cm.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)