* [patch] sctp: cleanup: remove unneeded null check @ 2010-04-23 11:59 Dan Carpenter 2010-04-23 14:28 ` Vlad Yasevich 0 siblings, 1 reply; 6+ messages in thread From: Dan Carpenter @ 2010-04-23 11:59 UTC (permalink / raw) To: Vlad Yasevich Cc: Sridhar Samudrala, David S. Miller, Wei Yongjun, Chris Dischino, linux-sctp, netdev, kernel-janitors "chunk" can never be null here. We dereferenced it earlier in the function and also at the start of the function we passed it to sctp_pack_cookie() which dereferences it. This code has been around since the dawn of git history so if "chunk" were ever null someone would have complained about it. Signed-off-by: Dan Carpenter <error27@gmail.com> diff --git a/net/sctp/sm_make_chunk.c b/net/sctp/sm_make_chunk.c index 17cb400..52352fc 100644 --- a/net/sctp/sm_make_chunk.c +++ b/net/sctp/sm_make_chunk.c @@ -470,8 +470,7 @@ struct sctp_chunk *sctp_make_init_ack(const struct sctp_association *asoc, * * [INIT ACK back to where the INIT came from.] */ - if (chunk) - retval->transport = chunk->transport; + retval->transport = chunk->transport; nomem_chunk: kfree(cookie); ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [patch] sctp: cleanup: remove unneeded null check 2010-04-23 11:59 [patch] sctp: cleanup: remove unneeded null check Dan Carpenter @ 2010-04-23 14:28 ` Vlad Yasevich 2010-04-24 17:19 ` [patch v2] sctp: cleanup: remove duplicate assignment Dan Carpenter 0 siblings, 1 reply; 6+ messages in thread From: Vlad Yasevich @ 2010-04-23 14:28 UTC (permalink / raw) To: Dan Carpenter Cc: Sridhar Samudrala, David S. Miller, Wei Yongjun, Chris Dischino, linux-sctp, netdev, kernel-janitors Dan Carpenter wrote: > "chunk" can never be null here. We dereferenced it earlier in the > function and also at the start of the function we passed it to > sctp_pack_cookie() which dereferences it. > > This code has been around since the dawn of git history so if "chunk" > were ever null someone would have complained about it. > > Signed-off-by: Dan Carpenter <error27@gmail.com> > > diff --git a/net/sctp/sm_make_chunk.c b/net/sctp/sm_make_chunk.c > index 17cb400..52352fc 100644 > --- a/net/sctp/sm_make_chunk.c > +++ b/net/sctp/sm_make_chunk.c > @@ -470,8 +470,7 @@ struct sctp_chunk *sctp_make_init_ack(const struct sctp_association *asoc, > * > * [INIT ACK back to where the INIT came from.] > */ > - if (chunk) > - retval->transport = chunk->transport; > + retval->transport = chunk->transport; > Actually, this code can be completely removed as we already make this assignment earlier: /* Per the advice in RFC 2960 6.4, send this reply to * the source of the INIT packet. */ retval->transport = chunk->transport; retval->subh.init_hdr = sctp_addto_chunk(retval, sizeof(initack), &initack); -vlad > nomem_chunk: > kfree(cookie); > -- > To unsubscribe from this list: send the line "unsubscribe linux-sctp" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > ^ permalink raw reply [flat|nested] 6+ messages in thread
* [patch v2] sctp: cleanup: remove duplicate assignment 2010-04-23 14:28 ` Vlad Yasevich @ 2010-04-24 17:19 ` Dan Carpenter 2010-04-27 14:32 ` Vlad Yasevich 0 siblings, 1 reply; 6+ messages in thread From: Dan Carpenter @ 2010-04-24 17:19 UTC (permalink / raw) To: Vlad Yasevich Cc: Sridhar Samudrala, David S. Miller, Wei Yongjun, Chris Dischino, linux-sctp, netdev, kernel-janitors This assignment isn't needed because we did it earlier already. Also another reason to delete the assignment is because it triggers a Smatch warning about checking for NULL pointers after a dereference. Reported-by: Vlad Yasevich <vladislav.yasevich@hp.com> Signed-off-by: Dan Carpenter <error27@gmail.com> --- Thanks Vlad. I came so close to seeing that myself if only I had openned my eyes a tiny bit more. :P diff --git a/net/sctp/sm_make_chunk.c b/net/sctp/sm_make_chunk.c index 17cb400..33aed1c 100644 --- a/net/sctp/sm_make_chunk.c +++ b/net/sctp/sm_make_chunk.c @@ -419,10 +419,17 @@ struct sctp_chunk *sctp_make_init_ack(const struct sctp_association *asoc, if (!retval) goto nomem_chunk; - /* Per the advice in RFC 2960 6.4, send this reply to - * the source of the INIT packet. + /* RFC 2960 6.4 Multi-homed SCTP Endpoints + * + * An endpoint SHOULD transmit reply chunks (e.g., SACK, + * HEARTBEAT ACK, * etc.) to the same destination transport + * address from which it received the DATA or control chunk + * to which it is replying. + * + * [INIT ACK back to where the INIT came from.] */ retval->transport = chunk->transport; + retval->subh.init_hdr = sctp_addto_chunk(retval, sizeof(initack), &initack); retval->param_hdr.v = sctp_addto_chunk(retval, addrs_len, addrs.v); @@ -461,18 +468,6 @@ struct sctp_chunk *sctp_make_init_ack(const struct sctp_association *asoc, /* We need to remove the const qualifier at this point. */ retval->asoc = (struct sctp_association *) asoc; - /* RFC 2960 6.4 Multi-homed SCTP Endpoints - * - * An endpoint SHOULD transmit reply chunks (e.g., SACK, - * HEARTBEAT ACK, * etc.) to the same destination transport - * address from which it received the DATA or control chunk - * to which it is replying. - * - * [INIT ACK back to where the INIT came from.] - */ - if (chunk) - retval->transport = chunk->transport; - nomem_chunk: kfree(cookie); nomem_cookie: ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [patch v2] sctp: cleanup: remove duplicate assignment 2010-04-24 17:19 ` [patch v2] sctp: cleanup: remove duplicate assignment Dan Carpenter @ 2010-04-27 14:32 ` Vlad Yasevich 2010-04-27 16:58 ` David Miller 0 siblings, 1 reply; 6+ messages in thread From: Vlad Yasevich @ 2010-04-27 14:32 UTC (permalink / raw) To: Dan Carpenter Cc: Sridhar Samudrala, David S. Miller, Wei Yongjun, Chris Dischino, linux-sctp, netdev, kernel-janitors Dan Carpenter wrote: > This assignment isn't needed because we did it earlier already. > > Also another reason to delete the assignment is because it triggers a > Smatch warning about checking for NULL pointers after a dereference. > > Reported-by: Vlad Yasevich <vladislav.yasevich@hp.com> > Signed-off-by: Dan Carpenter <error27@gmail.com> Thanks. I'll take this one. -vlad > --- > Thanks Vlad. I came so close to seeing that myself if only I had openned > my eyes a tiny bit more. :P > > diff --git a/net/sctp/sm_make_chunk.c b/net/sctp/sm_make_chunk.c > index 17cb400..33aed1c 100644 > --- a/net/sctp/sm_make_chunk.c > +++ b/net/sctp/sm_make_chunk.c > @@ -419,10 +419,17 @@ struct sctp_chunk *sctp_make_init_ack(const struct sctp_association *asoc, > if (!retval) > goto nomem_chunk; > > - /* Per the advice in RFC 2960 6.4, send this reply to > - * the source of the INIT packet. > + /* RFC 2960 6.4 Multi-homed SCTP Endpoints > + * > + * An endpoint SHOULD transmit reply chunks (e.g., SACK, > + * HEARTBEAT ACK, * etc.) to the same destination transport > + * address from which it received the DATA or control chunk > + * to which it is replying. > + * > + * [INIT ACK back to where the INIT came from.] > */ > retval->transport = chunk->transport; > + > retval->subh.init_hdr = > sctp_addto_chunk(retval, sizeof(initack), &initack); > retval->param_hdr.v = sctp_addto_chunk(retval, addrs_len, addrs.v); > @@ -461,18 +468,6 @@ struct sctp_chunk *sctp_make_init_ack(const struct sctp_association *asoc, > /* We need to remove the const qualifier at this point. */ > retval->asoc = (struct sctp_association *) asoc; > > - /* RFC 2960 6.4 Multi-homed SCTP Endpoints > - * > - * An endpoint SHOULD transmit reply chunks (e.g., SACK, > - * HEARTBEAT ACK, * etc.) to the same destination transport > - * address from which it received the DATA or control chunk > - * to which it is replying. > - * > - * [INIT ACK back to where the INIT came from.] > - */ > - if (chunk) > - retval->transport = chunk->transport; > - > nomem_chunk: > kfree(cookie); > nomem_cookie: > -- > To unsubscribe from this list: send the line "unsubscribe linux-sctp" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [patch v2] sctp: cleanup: remove duplicate assignment 2010-04-27 14:32 ` Vlad Yasevich @ 2010-04-27 16:58 ` David Miller 2010-04-27 17:32 ` Vlad Yasevich 0 siblings, 1 reply; 6+ messages in thread From: David Miller @ 2010-04-27 16:58 UTC (permalink / raw) To: vladislav.yasevich Cc: error27, sri, yjwei, cdischino, linux-sctp, netdev, kernel-janitors From: Vlad Yasevich <vladislav.yasevich@hp.com> Date: Tue, 27 Apr 2010 10:32:34 -0400 > > > Dan Carpenter wrote: >> This assignment isn't needed because we did it earlier already. >> >> Also another reason to delete the assignment is because it triggers a >> Smatch warning about checking for NULL pointers after a dereference. >> >> Reported-by: Vlad Yasevich <vladislav.yasevich@hp.com> >> Signed-off-by: Dan Carpenter <error27@gmail.com> > > Thanks. I'll take this one. And when will I get this from you? :-) ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [patch v2] sctp: cleanup: remove duplicate assignment 2010-04-27 16:58 ` David Miller @ 2010-04-27 17:32 ` Vlad Yasevich 0 siblings, 0 replies; 6+ messages in thread From: Vlad Yasevich @ 2010-04-27 17:32 UTC (permalink / raw) To: David Miller Cc: error27, sri, yjwei, cdischino, linux-sctp, netdev, kernel-janitors David Miller wrote: > From: Vlad Yasevich <vladislav.yasevich@hp.com> > Date: Tue, 27 Apr 2010 10:32:34 -0400 > >> >> Dan Carpenter wrote: >>> This assignment isn't needed because we did it earlier already. >>> >>> Also another reason to delete the assignment is because it triggers a >>> Smatch warning about checking for NULL pointers after a dereference. >>> >>> Reported-by: Vlad Yasevich <vladislav.yasevich@hp.com> >>> Signed-off-by: Dan Carpenter <error27@gmail.com> >> Thanks. I'll take this one. > > And when will I get this from you? :-) By the end of the week. I am trying to get all the testing finished. :) -vlad > -- > To unsubscribe from this list: send the line "unsubscribe linux-sctp" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2010-04-27 17:32 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2010-04-23 11:59 [patch] sctp: cleanup: remove unneeded null check Dan Carpenter 2010-04-23 14:28 ` Vlad Yasevich 2010-04-24 17:19 ` [patch v2] sctp: cleanup: remove duplicate assignment Dan Carpenter 2010-04-27 14:32 ` Vlad Yasevich 2010-04-27 16:58 ` David Miller 2010-04-27 17:32 ` Vlad Yasevich
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).