diff mbox series

[ovs-dev,12/13] conntrack: Fix dead assignment reported by clang.

Message ID 1504893565-110166-13-git-send-email-bhanuprakash.bodireddy@intel.com
State Superseded
Delegated to: Darrell Ball
Headers show
Series Rearrange structure members for memory efficiency. | expand

Commit Message

Bodireddy, Bhanuprakash Sept. 8, 2017, 5:59 p.m. UTC 
Clang reports that value stored to ftp, seq_skew_dir never read inside
the function.

Signed-off-by: Bhanuprakash Bodireddy <bhanuprakash.bodireddy@intel.com>
---
 lib/conntrack.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

Comments

Darrell Ball Sept. 8, 2017, 7:41 p.m. UTC | #1 
Hi Bhanu

What version of clang are you using and in what environment ?

Thanks Darrell


On 9/8/17, 10:59 AM, "ovs-dev-bounces@openvswitch.org on behalf of Bhanuprakash Bodireddy" <ovs-dev-bounces@openvswitch.org on behalf of bhanuprakash.bodireddy@intel.com> wrote:

    Clang reports that value stored to ftp, seq_skew_dir never read inside
    the function.
    
    Signed-off-by: Bhanuprakash Bodireddy <bhanuprakash.bodireddy@intel.com>
    ---
     lib/conntrack.c | 5 ++---
     1 file changed, 2 insertions(+), 3 deletions(-)
    
    diff --git a/lib/conntrack.c b/lib/conntrack.c
    index 419cb1d..a0838ee 100644
    --- a/lib/conntrack.c
    +++ b/lib/conntrack.c
    @@ -2615,7 +2615,7 @@ process_ftp_ctl_v4(struct conntrack *ct,
         char ftp_msg[LARGEST_FTP_MSG_OF_INTEREST + 1] = {0};
         get_ftp_ctl_msg(pkt, ftp_msg);
     
    -    char *ftp = ftp_msg;
    +    char *ftp;
         enum ct_alg_mode mode;
         if (!strncasecmp(ftp_msg, FTP_PORT_CMD, strlen(FTP_PORT_CMD))) {
             ftp = ftp_msg + strlen(FTP_PORT_CMD);
    @@ -2761,7 +2761,7 @@ process_ftp_ctl_v6(struct conntrack *ct,
         get_ftp_ctl_msg(pkt, ftp_msg);
         *ftp_data_start = tcp_hdr + tcp_hdr_len;
     
    -    char *ftp = ftp_msg;
    +    char *ftp;
         struct in6_addr ip6_addr;
         if (!strncasecmp(ftp_msg, FTP_EPRT_CMD, strlen(FTP_EPRT_CMD))) {
             ftp = ftp_msg + strlen(FTP_EPRT_CMD);
    @@ -2909,7 +2909,6 @@ handle_ftp_ctl(struct conntrack *ct, const struct conn_lookup_ctx *ctx,
         bool seq_skew_dir;
         if (ftp_ctl == CT_FTP_CTL_OTHER) {
             seq_skew = conn_for_expectation->seq_skew;
    -        seq_skew_dir = conn_for_expectation->seq_skew_dir;
         } else if (ftp_ctl == CT_FTP_CTL_INTEREST) {
             enum ftp_ctl_pkt rc;
             if (ctx->key.dl_type == htons(ETH_TYPE_IPV6)) {
    -- 
    2.4.11
    
    _______________________________________________
    dev mailing list
    dev@openvswitch.org
    https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-uZnsw&m=LE5PLIlvBSFThteUGJevgTlRlFesohyngSzGDqpvk5k&s=BsQfIBSohBfsM_UTvU-fZeE6EswgpKmd9tz0snT8usc&e=
Bodireddy, Bhanuprakash Sept. 10, 2017, 5:25 p.m. UTC | #2 
Hi Darrell,

>What version of clang are you using and in what environment ?

The clang version is  3.5.0. This was seen with clang static analysis.

- Bhanuprakash.

>
>On 9/8/17, 10:59 AM, "ovs-dev-bounces@openvswitch.org on behalf of
>Bhanuprakash Bodireddy" <ovs-dev-bounces@openvswitch.org on behalf of
>bhanuprakash.bodireddy@intel.com> wrote:
>
>    Clang reports that value stored to ftp, seq_skew_dir never read inside
>    the function.
>
>    Signed-off-by: Bhanuprakash Bodireddy
><bhanuprakash.bodireddy@intel.com>
>    ---
>     lib/conntrack.c | 5 ++---
>     1 file changed, 2 insertions(+), 3 deletions(-)
>
>    diff --git a/lib/conntrack.c b/lib/conntrack.c
>    index 419cb1d..a0838ee 100644
>    --- a/lib/conntrack.c
>    +++ b/lib/conntrack.c
>    @@ -2615,7 +2615,7 @@ process_ftp_ctl_v4(struct conntrack *ct,
>         char ftp_msg[LARGEST_FTP_MSG_OF_INTEREST + 1] = {0};
>         get_ftp_ctl_msg(pkt, ftp_msg);
>
>    -    char *ftp = ftp_msg;
>    +    char *ftp;
>         enum ct_alg_mode mode;
>         if (!strncasecmp(ftp_msg, FTP_PORT_CMD, strlen(FTP_PORT_CMD))) {
>             ftp = ftp_msg + strlen(FTP_PORT_CMD);
>    @@ -2761,7 +2761,7 @@ process_ftp_ctl_v6(struct conntrack *ct,
>         get_ftp_ctl_msg(pkt, ftp_msg);
>         *ftp_data_start = tcp_hdr + tcp_hdr_len;
>
>    -    char *ftp = ftp_msg;
>    +    char *ftp;
>         struct in6_addr ip6_addr;
>         if (!strncasecmp(ftp_msg, FTP_EPRT_CMD, strlen(FTP_EPRT_CMD))) {
>             ftp = ftp_msg + strlen(FTP_EPRT_CMD);
>    @@ -2909,7 +2909,6 @@ handle_ftp_ctl(struct conntrack *ct, const struct
>conn_lookup_ctx *ctx,
>         bool seq_skew_dir;
>         if (ftp_ctl == CT_FTP_CTL_OTHER) {
>             seq_skew = conn_for_expectation->seq_skew;
>    -        seq_skew_dir = conn_for_expectation->seq_skew_dir;
>         } else if (ftp_ctl == CT_FTP_CTL_INTEREST) {
>             enum ftp_ctl_pkt rc;
>             if (ctx->key.dl_type == htons(ETH_TYPE_IPV6)) {
>    --
>    2.4.11
>
>    _______________________________________________
>    dev mailing list
>    dev@openvswitch.org
>    https://urldefense.proofpoint.com/v2/url?u=https-
>3A__mail.openvswitch.org_mailman_listinfo_ovs-
>2Ddev&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-
>uZnsw&m=LE5PLIlvBSFThteUGJevgTlRlFesohyngSzGDqpvk5k&s=BsQfIBSohBf
>sM_UTvU-fZeE6EswgpKmd9tz0snT8usc&e=
>
Gregory Rose Sept. 15, 2017, 5:39 p.m. UTC | #3 
On 09/08/2017 10:59 AM, Bhanuprakash Bodireddy wrote:
> Clang reports that value stored to ftp, seq_skew_dir never read inside
> the function.
> 
> Signed-off-by: Bhanuprakash Bodireddy <bhanuprakash.bodireddy@intel.com>
> ---
>   lib/conntrack.c | 5 ++---
>   1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/lib/conntrack.c b/lib/conntrack.c
> index 419cb1d..a0838ee 100644
> --- a/lib/conntrack.c
> +++ b/lib/conntrack.c
> @@ -2615,7 +2615,7 @@ process_ftp_ctl_v4(struct conntrack *ct,
>       char ftp_msg[LARGEST_FTP_MSG_OF_INTEREST + 1] = {0};
>       get_ftp_ctl_msg(pkt, ftp_msg);
>   
> -    char *ftp = ftp_msg;
> +    char *ftp;
>       enum ct_alg_mode mode;
>       if (!strncasecmp(ftp_msg, FTP_PORT_CMD, strlen(FTP_PORT_CMD))) {
>           ftp = ftp_msg + strlen(FTP_PORT_CMD);
> @@ -2761,7 +2761,7 @@ process_ftp_ctl_v6(struct conntrack *ct,
>       get_ftp_ctl_msg(pkt, ftp_msg);
>       *ftp_data_start = tcp_hdr + tcp_hdr_len;
>   
> -    char *ftp = ftp_msg;
> +    char *ftp;
>       struct in6_addr ip6_addr;
>       if (!strncasecmp(ftp_msg, FTP_EPRT_CMD, strlen(FTP_EPRT_CMD))) {
>           ftp = ftp_msg + strlen(FTP_EPRT_CMD);
> @@ -2909,7 +2909,6 @@ handle_ftp_ctl(struct conntrack *ct, const struct conn_lookup_ctx *ctx,
>       bool seq_skew_dir;
>       if (ftp_ctl == CT_FTP_CTL_OTHER) {
>           seq_skew = conn_for_expectation->seq_skew;
> -        seq_skew_dir = conn_for_expectation->seq_skew_dir;
>       } else if (ftp_ctl == CT_FTP_CTL_INTEREST) {
>           enum ftp_ctl_pkt rc;
>           if (ctx->key.dl_type == htons(ETH_TYPE_IPV6)) {
> 

Looks good.

Reviewed-by: Greg Rose <gvrose8192@gmail.com>
Darrell Ball Sept. 19, 2017, 8:29 p.m. UTC | #4 
Thanks Bhanu

It seems my scan-build usage was not catching these.
I decided to do something a little different which I’ll send out later today as part of some other changes

Darrell

On 9/10/17, 10:25 AM, "Bodireddy, Bhanuprakash" <bhanuprakash.bodireddy@intel.com> wrote:

    Hi Darrell,
    
    >What version of clang are you using and in what environment ?

    
    The clang version is  3.5.0. This was seen with clang static analysis.
    
    - Bhanuprakash.
    
    >

    >On 9/8/17, 10:59 AM, "ovs-dev-bounces@openvswitch.org on behalf of

    >Bhanuprakash Bodireddy" <ovs-dev-bounces@openvswitch.org on behalf of

    >bhanuprakash.bodireddy@intel.com> wrote:

    >

    >    Clang reports that value stored to ftp, seq_skew_dir never read inside

    >    the function.

    >

    >    Signed-off-by: Bhanuprakash Bodireddy

    ><bhanuprakash.bodireddy@intel.com>

    >    ---

    >     lib/conntrack.c | 5 ++---

    >     1 file changed, 2 insertions(+), 3 deletions(-)

    >

    >    diff --git a/lib/conntrack.c b/lib/conntrack.c

    >    index 419cb1d..a0838ee 100644

    >    --- a/lib/conntrack.c

    >    +++ b/lib/conntrack.c

    >    @@ -2615,7 +2615,7 @@ process_ftp_ctl_v4(struct conntrack *ct,

    >         char ftp_msg[LARGEST_FTP_MSG_OF_INTEREST + 1] = {0};

    >         get_ftp_ctl_msg(pkt, ftp_msg);

    >

    >    -    char *ftp = ftp_msg;

    >    +    char *ftp;

    >         enum ct_alg_mode mode;

    >         if (!strncasecmp(ftp_msg, FTP_PORT_CMD, strlen(FTP_PORT_CMD))) {

    >             ftp = ftp_msg + strlen(FTP_PORT_CMD);

    >    @@ -2761,7 +2761,7 @@ process_ftp_ctl_v6(struct conntrack *ct,

    >         get_ftp_ctl_msg(pkt, ftp_msg);

    >         *ftp_data_start = tcp_hdr + tcp_hdr_len;

    >

    >    -    char *ftp = ftp_msg;

    >    +    char *ftp;

    >         struct in6_addr ip6_addr;

    >         if (!strncasecmp(ftp_msg, FTP_EPRT_CMD, strlen(FTP_EPRT_CMD))) {

    >             ftp = ftp_msg + strlen(FTP_EPRT_CMD);

    >    @@ -2909,7 +2909,6 @@ handle_ftp_ctl(struct conntrack *ct, const struct

    >conn_lookup_ctx *ctx,

    >         bool seq_skew_dir;

    >         if (ftp_ctl == CT_FTP_CTL_OTHER) {

    >             seq_skew = conn_for_expectation->seq_skew;

    >    -        seq_skew_dir = conn_for_expectation->seq_skew_dir;

    >         } else if (ftp_ctl == CT_FTP_CTL_INTEREST) {

    >             enum ftp_ctl_pkt rc;

    >             if (ctx->key.dl_type == htons(ETH_TYPE_IPV6)) {

    >    --

    >    2.4.11

    >

    >    _______________________________________________

    >    dev mailing list

    >    dev@openvswitch.org

    >    https://urldefense.proofpoint.com/v2/url?u=https-

    >3A__mail.openvswitch.org_mailman_listinfo_ovs-

    >2Ddev&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-

    >uZnsw&m=LE5PLIlvBSFThteUGJevgTlRlFesohyngSzGDqpvk5k&s=BsQfIBSohBf

    >sM_UTvU-fZeE6EswgpKmd9tz0snT8usc&e=

    >
Darrell Ball Sept. 20, 2017, 4:09 a.m. UTC | #5 
Hi Bhanu

Thanks for the report.
I sent a different patch here:
https://patchwork.ozlabs.org/patch/815998/

Pls let me know if it makes sense and more importantly if it does not.

Thanks Darrell

On 9/19/17, 1:29 PM, "Darrell Ball" <dball@vmware.com> wrote:

    Thanks Bhanu
    
    It seems my scan-build usage was not catching these.
    I decided to do something a little different which I’ll send out later today as part of some other changes
    
    Darrell
    
    On 9/10/17, 10:25 AM, "Bodireddy, Bhanuprakash" <bhanuprakash.bodireddy@intel.com> wrote:
    
        Hi Darrell,
        
        >What version of clang are you using and in what environment ?

        
        The clang version is  3.5.0. This was seen with clang static analysis.
        
        - Bhanuprakash.
        
        >

        >On 9/8/17, 10:59 AM, "ovs-dev-bounces@openvswitch.org on behalf of

        >Bhanuprakash Bodireddy" <ovs-dev-bounces@openvswitch.org on behalf of

        >bhanuprakash.bodireddy@intel.com> wrote:

        >

        >    Clang reports that value stored to ftp, seq_skew_dir never read inside

        >    the function.

        >

        >    Signed-off-by: Bhanuprakash Bodireddy

        ><bhanuprakash.bodireddy@intel.com>

        >    ---

        >     lib/conntrack.c | 5 ++---

        >     1 file changed, 2 insertions(+), 3 deletions(-)

        >

        >    diff --git a/lib/conntrack.c b/lib/conntrack.c

        >    index 419cb1d..a0838ee 100644

        >    --- a/lib/conntrack.c

        >    +++ b/lib/conntrack.c

        >    @@ -2615,7 +2615,7 @@ process_ftp_ctl_v4(struct conntrack *ct,

        >         char ftp_msg[LARGEST_FTP_MSG_OF_INTEREST + 1] = {0};

        >         get_ftp_ctl_msg(pkt, ftp_msg);

        >

        >    -    char *ftp = ftp_msg;

        >    +    char *ftp;

        >         enum ct_alg_mode mode;

        >         if (!strncasecmp(ftp_msg, FTP_PORT_CMD, strlen(FTP_PORT_CMD))) {

        >             ftp = ftp_msg + strlen(FTP_PORT_CMD);

        >    @@ -2761,7 +2761,7 @@ process_ftp_ctl_v6(struct conntrack *ct,

        >         get_ftp_ctl_msg(pkt, ftp_msg);

        >         *ftp_data_start = tcp_hdr + tcp_hdr_len;

        >

        >    -    char *ftp = ftp_msg;

        >    +    char *ftp;

        >         struct in6_addr ip6_addr;

        >         if (!strncasecmp(ftp_msg, FTP_EPRT_CMD, strlen(FTP_EPRT_CMD))) {

        >             ftp = ftp_msg + strlen(FTP_EPRT_CMD);

        >    @@ -2909,7 +2909,6 @@ handle_ftp_ctl(struct conntrack *ct, const struct

        >conn_lookup_ctx *ctx,

        >         bool seq_skew_dir;

        >         if (ftp_ctl == CT_FTP_CTL_OTHER) {

        >             seq_skew = conn_for_expectation->seq_skew;

        >    -        seq_skew_dir = conn_for_expectation->seq_skew_dir;

        >         } else if (ftp_ctl == CT_FTP_CTL_INTEREST) {

        >             enum ftp_ctl_pkt rc;

        >             if (ctx->key.dl_type == htons(ETH_TYPE_IPV6)) {

        >    --

        >    2.4.11

        >

        >    _______________________________________________

        >    dev mailing list

        >    dev@openvswitch.org

        >    https://urldefense.proofpoint.com/v2/url?u=https-

        >3A__mail.openvswitch.org_mailman_listinfo_ovs-

        >2Ddev&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-

        >uZnsw&m=LE5PLIlvBSFThteUGJevgTlRlFesohyngSzGDqpvk5k&s=BsQfIBSohBf

        >sM_UTvU-fZeE6EswgpKmd9tz0snT8usc&e=

        >
diff mbox series

Patch

diff --git a/lib/conntrack.c b/lib/conntrack.c
index 419cb1d..a0838ee 100644
--- a/lib/conntrack.c
+++ b/lib/conntrack.c
@@ -2615,7 +2615,7 @@  process_ftp_ctl_v4(struct conntrack *ct,
     char ftp_msg[LARGEST_FTP_MSG_OF_INTEREST + 1] = {0};
     get_ftp_ctl_msg(pkt, ftp_msg);
 
-    char *ftp = ftp_msg;
+    char *ftp;
     enum ct_alg_mode mode;
     if (!strncasecmp(ftp_msg, FTP_PORT_CMD, strlen(FTP_PORT_CMD))) {
         ftp = ftp_msg + strlen(FTP_PORT_CMD);
@@ -2761,7 +2761,7 @@  process_ftp_ctl_v6(struct conntrack *ct,
     get_ftp_ctl_msg(pkt, ftp_msg);
     *ftp_data_start = tcp_hdr + tcp_hdr_len;
 
-    char *ftp = ftp_msg;
+    char *ftp;
     struct in6_addr ip6_addr;
     if (!strncasecmp(ftp_msg, FTP_EPRT_CMD, strlen(FTP_EPRT_CMD))) {
         ftp = ftp_msg + strlen(FTP_EPRT_CMD);
@@ -2909,7 +2909,6 @@  handle_ftp_ctl(struct conntrack *ct, const struct conn_lookup_ctx *ctx,
     bool seq_skew_dir;
     if (ftp_ctl == CT_FTP_CTL_OTHER) {
         seq_skew = conn_for_expectation->seq_skew;
-        seq_skew_dir = conn_for_expectation->seq_skew_dir;
     } else if (ftp_ctl == CT_FTP_CTL_INTEREST) {
         enum ftp_ctl_pkt rc;
         if (ctx->key.dl_type == htons(ETH_TYPE_IPV6)) {