Skip to content

Fix a few gcc -fanalyzer warnings#120

Merged
troglobit merged 3 commits intolibnet:masterfrom
adrianreber:master
Apr 23, 2021
Merged

Fix a few gcc -fanalyzer warnings#120
troglobit merged 3 commits intolibnet:masterfrom
adrianreber:master

Conversation

@adrianreber
Copy link
Contributor

I tried to compile libnet with -fanalyzer and tried to fix a few of the warnings.

There are more warnings, but it seems a couple of them are false positives. The warnings fixed in this PR seemed correct, however.

This fixes:

libnet_port_list.c:99:8: warning: leak of ‘_8’ [CWE-401] [-Wanalyzer-malloc-leak]
   99 |     if (!all_lists)
      |        ^
  ‘libnet_plist_chain_new’: events 1-3
    |
    |   48 |     if (l == NULL)
    |      |        ^
    |      |        |
    |      |        (1) following ‘false’ branch (when ‘l_70(D)’ is non-NULL)...
    |......
    |   53 |     if (token_list == NULL)
    |      |     ~~ ~
    |      |     |  |
    |      |     |  (3) following ‘false’ branch (when ‘token_list_71(D)’ is non-NULL)...
    |      |     (2) ...to here
    |
  ‘libnet_plist_chain_new’: event 4
    |
    |cc1:
    | (4): ...to here
    |
  ‘libnet_plist_chain_new’: events 5-9
    |
    |   83 |     *plist = malloc(sizeof (libnet_plist_t));
    |      |              ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    |      |              |
    |      |              (5) allocated here
    |   84 |
    |   85 |     if (!(*plist))
    |      |        ~
    |      |        |
    |      |        (6) assuming ‘*plist_74(D)’ is non-NULL
    |      |        (7) following ‘false’ branch...
    |......
    |   93 |     tmp = *plist;
    |      |     ~~~
    |      |     |
    |      |     (8) ...to here
    |......
    |   99 |     if (!all_lists)
    |      |        ~
    |      |        |
    |      |        (9) ‘_8’ leaks here; was allocated at (5)
    |

Signed-off-by: Adrian Reber <areber@redhat.com>
Signed-off-by: Adrian Reber <areber@redhat.com>
libnet_cq.c:139:18: warning: dereference of possibly-NULL ‘new_18’ [CWE-690] [-Wanalyzer-possible-null-dereference]
  139 |     new->context = l;
      |     ~~~~~~~~~~~~~^~~
  ‘libnet_cq_add.part.0’: events 1-6
    |
    |   71 | libnet_cq_add(libnet_t *l, char *label)
    |      | ^~~~~~~~~~~~~
    |      | |
    |      | (1) entry to ‘libnet_cq_add.part.0’
    |......
    |   89 |     if (label == NULL)
    |      |        ~
    |      |        |
    |      |        (2) following ‘false’ branch (when ‘label_1(D)’ is non-NULL)...
    |......
    |   97 |     if (l_cq == NULL)
    |      |     ~~ ~
    |      |     |  |
    |      |     |  (4) following ‘false’ branch...
    |      |     (3) ...to here
    |......
    |  124 |     if (libnet_cq_dup_check(l, label))
    |      |     ~~  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    |      |     |   |
    |      |     |   (6) calling ‘libnet_cq_dup_check’ from ‘libnet_cq_add.part.0’
    |      |     (5) ...to here
    |
    +--> ‘libnet_cq_dup_check’: events 7-13
           |
           |  269 | libnet_cq_dup_check(libnet_t *l, char *label)
           |      | ^~~~~~~~~~~~~~~~~~~
           |      | |
           |      | (7) entry to ‘libnet_cq_dup_check’
           |......
           |  273 |     for (p = l_cq; p; p = p->next)
           |      |                    ~  ~~~~~~~~~~~
           |      |                    |    |
           |      |                    |    (13) ...to here
           |      |                    (8) following ‘true’ branch (when ‘p_6’ is non-NULL)...
           |  274 |     {
           |  275 |         if (p->context == l)
           |      |         ~~ ~
           |      |         |  |
           |      |         |  (10) following ‘false’ branch...
           |      |         (9) ...to here
           |......
           |  281 |         if (strncmp(p->context->label, label, LIBNET_LABEL_SIZE) == 0)
           |      |         ~~ ~
           |      |         |  |
           |      |         |  (12) following ‘false’ branch...
           |      |         (11) ...to here
           |
    <------+
    |
  ‘libnet_cq_add.part.0’: events 14-20
    |
    |  124 |     if (libnet_cq_dup_check(l, label))
    |      |        ~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    |      |        ||
    |      |        |(14) returning to ‘libnet_cq_add.part.0’ from ‘libnet_cq_dup_check’
    |      |        (15) following ‘false’ branch...
    |......
    |  130 |     new = (libnet_cq_t *)malloc(sizeof (libnet_cq_t));
    |      |     ~~~                  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    |      |     |                    |
    |      |     |                    (17) this call could return NULL
    |      |     (16) ...to here
    |  131 |     if (l_cq == NULL)
    |      |        ~
    |      |        |
    |      |        (18) following ‘false’ branch...
    |......
    |  139 |     new->context = l;
    |      |     ~~~~~~~~~~~~~~~~
    |      |     |            |
    |      |     |            (20) ‘new_18’ could be NULL: unchecked value from (17)
    |      |     (19) ...to here
    |

Signed-off-by: Adrian Reber <areber@redhat.com>
@troglobit
Copy link
Collaborator

Awesome, great fixes, thank you! <3

@troglobit troglobit merged commit 1dba1f6 into libnet:master Apr 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants