GitLab

We got two reports from coverity about the HS circuitmap unittests (#21889 (moved)) which were merged in 0.3.1.

First one:

*** CID 1405129:    (NEGATIVE_RETURNS)
/src/test/test_hs_intropoint.c: 780 in test_received_introduce1_handling()
774       }
775     
776       /* Valid case. */
777       {
778         cell = helper_create_introduce1_cell();
779         ssize_t request_len = trn_cell_introduce1_encoded_len(cell);
>>>     CID 1405129:    (NEGATIVE_RETURNS)
>>>     Assigning: unsigned variable "print_" = "print1_".
780         tt_size_op(request_len, OP_GT, 0);

which is caused because we use tt_size_op to compare a ssize_t.

---

*** CID 1405130:    (REVERSE_INULL)
/src/test/test_circuitlist.c: 440 in test_hs_circuitmap_isolation()
434          * that token. */
435         tt_ptr_op(circ4, OP_EQ,
436                   hs_circuitmap_get_intro_circ_v2_service_side(tok2));
437       }
438     
439      done:
>>>     CID 1405130:    (REVERSE_INULL)
>>>     Null-checking "circ1" suggests that it may be null, but it has already been dereferenced on all paths leading to the check.
440       if (circ1)
441         circuit_free(TO_CIRCUIT(circ1));

which I think is caused because we NULL check circ1, even tho previous code is dereferencing it without any checks. Not sure what the right fix is here; perhaps we dont really need to NULL check it, and we can add a tt_assert(circ1) on the top as well.