Opened 6 days ago

Last modified 5 hours ago

#26780 merge_ready defect

Fix division by zero error reports from Coverity (CID: 1415721, 1415722, and 1415723)

Reported by: ahf Owned by:
Priority: Medium Milestone: Tor: unspecified
Component: Core Tor/Tor Version: Tor: unspecified
Severity: Normal Keywords:
Cc: Actual Points:
Parent ID: Points:
Reviewer: asn Sponsor:

Description

We should fix the current Coverity issues. Coverity CID 1415721, 1415722, and 1415723 seems to be somewhat related in that they all stem from Coverity not being able to identify the output domain for get_net_param_from_list()

Child Tickets

Change History (5)

comment:1 Changed 6 days ago by ahf

comment:2 Changed 6 days ago by ahf

Status: newneeds_review

Looks like 1407.6 has been failing for a while on Travis. Marking this needs review anyway.

comment:3 Changed 3 days ago by dgoulet

Reviewer: asn

comment:4 Changed 6 hours ago by asn

Status: needs_reviewmerge_ready

Changes LGTM.

I checked the coverity reports and I have no idea if the patch will fix them. I already imagine that coverity would not warn because of:

  if (res < min_val) {
    log_warn(LD_DIR, "Consensus parameter %s is too small. Got %d, raising to "
             "%d.", param_name, res, min_val);
    res = min_val;
  } else if (res > max_val) {
    log_warn(LD_DIR, "Consensus parameter %s is too large. Got %d, capping to "
             "%d.", param_name, res, max_val);
    res = max_val;
  }

Perhaps let's get these asserts merged, and check coverity again? :)

comment:5 Changed 5 hours ago by nickm

Fine with me -- merged to master. Please close this if the coverity warnings go away.

Note: See TracTickets for help on using tickets.