| Message ID | 556C2438.3060203@brocade.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Pablo Neira |
| Headers | show |
On Mon, Jun 01, 2015 at 10:22:00AM +0100, Paul Aitken wrote: > ct and myct have both already been checked for non-NULL, > so there's no need to check either of them again later. > > Signed-off-by: Paul Aitken <paitken@brocade.com> > --- > src/cthelper.c | 8 +++----- > 1 file changed, 3 insertions(+), 5 deletions(-) > > diff --git a/src/cthelper.c b/src/cthelper.c > index 15d5126..6537515 100644 > --- a/src/cthelper.c > +++ b/src/cthelper.c > @@ -325,14 +325,12 @@ static int nfq_queue_cb(const struct nlmsghdr *nlh, void *data) > if (pkt_verdict_issue(helper, myct, queue_num, id, verdict, pktb) < 0) > goto err_pktb; > - if (ct != NULL) > - nfct_destroy(ct); > + nfct_destroy(ct); void nfct_destroy(struct nf_conntrack *ct) { assert(ct != NULL); ... the library doesn't allow NULL pointers. > if (myct->exp != NULL) > nfexp_destroy(myct->exp); > - if (myct && myct->priv_data != NULL) > + if (myct->priv_data != NULL) > free(myct->priv_data); > - if (myct != NULL) > - free(myct); > + free(myct); > return MNL_CB_OK; > err_pktb: > -- > 1.9.1 > -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Pablo, > On Mon, Jun 01, 2015 at 10:22:00AM +0100, Paul Aitken wrote: >> ct and myct have both already been checked for non-NULL, >> so there's no need to check either of them again later. >> >> Signed-off-by: Paul Aitken <paitken@brocade.com> >> --- >> src/cthelper.c | 8 +++----- >> 1 file changed, 3 insertions(+), 5 deletions(-) >> >> diff --git a/src/cthelper.c b/src/cthelper.c >> index 15d5126..6537515 100644 >> --- a/src/cthelper.c >> +++ b/src/cthelper.c >> @@ -325,14 +325,12 @@ static int nfq_queue_cb(const struct nlmsghdr *nlh, void *data) >> if (pkt_verdict_issue(helper, myct, queue_num, id, verdict, pktb) < 0) >> goto err_pktb; >> - if (ct != NULL) >> - nfct_destroy(ct); >> + nfct_destroy(ct); > void nfct_destroy(struct nf_conntrack *ct) > { > assert(ct != NULL); > ... > > the library doesn't allow NULL pointers. ct was already checked for non-NULL when it was assigned around line 297: ct = nfct_new(); if (ct == NULL) goto err; - so ct cannot be NULL at line 325. P. >> if (myct->exp != NULL) >> nfexp_destroy(myct->exp); >> - if (myct && myct->priv_data != NULL) >> + if (myct->priv_data != NULL) >> free(myct->priv_data); >> - if (myct != NULL) >> - free(myct); >> + free(myct); >> return MNL_CB_OK; >> err_pktb: >> -- >> 1.9.1 >> -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Jun 01, 2015 at 12:19:20PM +0100, Paul Aitken wrote: > Pablo, > > >On Mon, Jun 01, 2015 at 10:22:00AM +0100, Paul Aitken wrote: > >>ct and myct have both already been checked for non-NULL, > >>so there's no need to check either of them again later. > >> > >>Signed-off-by: Paul Aitken <paitken@brocade.com> > >>--- > >> src/cthelper.c | 8 +++----- > >> 1 file changed, 3 insertions(+), 5 deletions(-) > >> > >>diff --git a/src/cthelper.c b/src/cthelper.c > >>index 15d5126..6537515 100644 > >>--- a/src/cthelper.c > >>+++ b/src/cthelper.c > >>@@ -325,14 +325,12 @@ static int nfq_queue_cb(const struct nlmsghdr *nlh, void *data) > >> if (pkt_verdict_issue(helper, myct, queue_num, id, verdict, pktb) < 0) > >> goto err_pktb; > >>- if (ct != NULL) > >>- nfct_destroy(ct); > >>+ nfct_destroy(ct); > >void nfct_destroy(struct nf_conntrack *ct) > >{ > > assert(ct != NULL); > > ... > > > >the library doesn't allow NULL pointers. > > ct was already checked for non-NULL when it was assigned around line 297: > > ct = nfct_new(); > if (ct == NULL) > goto err; > > - so ct cannot be NULL at line 325. Right, makes sense. However, your patch doesn't apply here for some reason: $ git am Optimise-nfq_queue_cb.patch Applying: Optimise nfq_queue_cb error: patch failed: src/cthelper.c:325 error: src/cthelper.c: patch does not apply Patch failed at 0001 Optimise nfq_queue_cb When you have resolved this problem run "git am --resolved". If you would prefer to skip this patch, instead run "git am --skip". To restore the original branch and stop patching run "git am --abort" -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Pablo, somehow tabs had been converted to spaces. Let me try again... P. On 01/06/15 18:34, Pablo Neira Ayuso wrote: > On Mon, Jun 01, 2015 at 12:19:20PM +0100, Paul Aitken wrote: >> Pablo, >> >>> On Mon, Jun 01, 2015 at 10:22:00AM +0100, Paul Aitken wrote: >>>> ct and myct have both already been checked for non-NULL, >>>> so there's no need to check either of them again later. >>>> >>>> Signed-off-by: Paul Aitken <paitken@brocade.com> >>>> --- >>>> src/cthelper.c | 8 +++----- >>>> 1 file changed, 3 insertions(+), 5 deletions(-) >>>> >>>> diff --git a/src/cthelper.c b/src/cthelper.c >>>> index 15d5126..6537515 100644 >>>> --- a/src/cthelper.c >>>> +++ b/src/cthelper.c >>>> @@ -325,14 +325,12 @@ static int nfq_queue_cb(const struct nlmsghdr *nlh, void *data) >>>> if (pkt_verdict_issue(helper, myct, queue_num, id, verdict, pktb) < 0) >>>> goto err_pktb; >>>> - if (ct != NULL) >>>> - nfct_destroy(ct); >>>> + nfct_destroy(ct); >>> void nfct_destroy(struct nf_conntrack *ct) >>> { >>> assert(ct != NULL); >>> ... >>> >>> the library doesn't allow NULL pointers. >> ct was already checked for non-NULL when it was assigned around line 297: >> >> ct = nfct_new(); >> if (ct == NULL) >> goto err; >> >> - so ct cannot be NULL at line 325. > Right, makes sense. However, your patch doesn't apply here for some > reason: > > $ git am Optimise-nfq_queue_cb.patch > Applying: Optimise nfq_queue_cb > error: patch failed: src/cthelper.c:325 > error: src/cthelper.c: patch does not apply > Patch failed at 0001 Optimise nfq_queue_cb > When you have resolved this problem run "git am --resolved". > If you would prefer to skip this patch, instead run "git am --skip". > To restore the original branch and stop patching run "git am --abort" -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/src/cthelper.c b/src/cthelper.c index 15d5126..6537515 100644 --- a/src/cthelper.c +++ b/src/cthelper.c @@ -325,14 +325,12 @@ static int nfq_queue_cb(const struct nlmsghdr *nlh, void *data) if (pkt_verdict_issue(helper, myct, queue_num, id, verdict, pktb) < 0) goto err_pktb; - if (ct != NULL) - nfct_destroy(ct); + nfct_destroy(ct); if (myct->exp != NULL) nfexp_destroy(myct->exp); - if (myct && myct->priv_data != NULL) + if (myct->priv_data != NULL) free(myct->priv_data); - if (myct != NULL) - free(myct); + free(myct); return MNL_CB_OK; err_pktb:
ct and myct have both already been checked for non-NULL, so there's no need to check either of them again later. Signed-off-by: Paul Aitken <paitken@brocade.com> --- src/cthelper.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-)