Refactor

[lePici]

Report

Argument cannot be negative

• related to the socket descriptor as a parameter in the close function. Coverity complains about the possible negative value of the descriptor.

Bad bit shift operation

ifc_tls.c

• macro FD_SET() in file libtrap/src/ifc_tls.c must not perform a shift operation with a negative value, otherwise undefined behavior will occur.
• Please consider whether a command should be added that prints a warning or if this solution is OK at all.

ip_prefix_search.c

• A bit shift operation has a shift amount which is too large. There is a risk of undefined behavior.
• It is possible that a bug was discovered, because the regularity of the mask is strange.

Try it yourself with this code (compile with gcc with -std=c99) and uncomment first print block:

#include <stdio.h>
#include <stdint.h>

void printBits(size_t const size, void const * const ptr)
{
   unsigned char *b = (unsigned char*) ptr;
   unsigned char byte;
   int i, j;

   for (i = size-1; i >= 0; i--) {
      for (j = 7; j >= 0; j--) {
         byte = (b[i] >> j) & 1;
         printf("%u", byte);
      }
   }
}

static void test()
{
   for(int i = 0; i < 129; i++) {
      // OLD SHIFTS
      uint32_t value1a = 0xFFFFFFFF>>(i > 31 ? 0 : 32 - i);
      uint32_t value2a = 0xFFFFFFFF>>(i > 63 ? 0 : (i > 32 ? 64 - i: 32));
      uint32_t value3a = 0xFFFFFFFF>>(i > 95 ? 0 : (i > 64 ? 96 - i: 32));
      uint32_t value4a = 0xFFFFFFFF>>(i > 127 ? 0 : (i > 96 ? 128 - i : 32));

      // REFACTORED SHIFTS
      uint32_t value1b = 0xFFFFFFFF >> (i == 0 || i >= 32 ? 0 : 32  - i);
      uint32_t value2b = i <= 32 ? 0 : 0xFFFFFFFF >> (i >= 64 ? 0 : 64  - i);
      uint32_t value3b = i <= 64 ? 0 : 0xFFFFFFFF >> (i >= 96 ? 0 : 96  - i);
      uint32_t value4b = i <= 96 ? 0 : 0xFFFFFFFF >> (128 - i);

      // UNCOMMENT IF YOU WANT SEE ORIGINAL RESULTS
      /*
      printBits(sizeof(uint32_t), &value1a);
      printf(" ");
      printBits(sizeof(uint32_t), &value2a);
      printf(" ");
      printBits(sizeof(uint32_t), &value3a);
      printf(" ");
      printBits(sizeof(uint32_t), &value4a);
      */

      //printf("\n------------------------------------------------\n");

      // UNCOMMENT IF YOU WANT CHECK NEW RESULTS
      /*
      printBits(sizeof(uint32_t), &value1b);
      printf(" ");
      printBits(sizeof(uint32_t), &value2b);
      printf(" ");
      printBits(sizeof(uint32_t), &value3b);
      printf(" ");
      printBits(sizeof(uint32_t), &value4b);
      */

      //puts("");

      if(value1a != value1b) {
         printf("%d error- %u vs %u\n", i, value1a, value1b);
      } else if(value2a != value2b) {
         printf("%d error- %u vs %u\n", i, value2a, value2b);
      } else if(value3a != value3b) {
         printf("%d error- %u vs %u\n", i, value3a, value3b);
      } else if(value4a != value4b) {
         printf("%d error- %u vs %u\n", i, value4a, value4b);
      }
   }
}

int main()
{
   test();
   return 0;
}

Resource leak, Double free

• The ip_prefix_search module does not free memory in the event of an error. It may not be serious, but I think that these problems shouldn't be in the Nemea-Framework.

Unchecked return value

• Please see libtrap/src/ifc_file.c:543 if this solution is OK, or you will need to modify the return type of the switch_file_wrapper function to respond to an error in the switch_file function

Other small mistakes

• Extra argument to printf format specifier
• Dereference before null check
• Buffer not null terminated
• Invalid type in argument to printf format specifier

[codecov-io]

Codecov Report

Merging #162 (1ee8618) into master (535cc6f) will decrease coverage by 0.07%. The diff coverage is 27.58%.

@@            Coverage Diff             @@
##           master     #162      +/-   ##
==========================================
- Coverage   41.95%   41.88%   -0.08%     
==========================================
  Files          73       73              
  Lines       14767    14804      +37     
==========================================
+ Hits         6195     6200       +5     
- Misses       8572     8604      +32     

Impacted Files	Coverage Δ
libtrap/src/ifc_file.c	48.83% <0.00%> (-0.13%)	:arrow_down:
libtrap/src/ifc_tcpip.c	57.34% <0.00%> (-0.14%)	:arrow_down:
libtrap/src/third-party/libjansson/load.c	0.00% <0.00%> (ø)
libtrap/tests/test_echo_ctx.c	0.00% <0.00%> (ø)
unirec/unirec.c	49.51% <0.00%> (-0.06%)	:arrow_down:
libtrap/src/trap.c	44.85% <25.00%> (-0.06%)	:arrow_down:
unirec/ip_prefix_search.c	69.71% <25.00%> (-2.96%)	:arrow_down:
libtrap/tests/test_echo.c	72.30% <50.00%> (-2.30%)	:arrow_down:
libtrap/tests/test_rxtx.c	70.78% <50.00%> (-1.31%)	:arrow_down:
libtrap/src/ifc_tls.c	56.85% <66.66%> (+0.02%)	:arrow_up:

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 535cc6f...1ee8618. Read the comment docs.